Skip to content

Enabling graceful shutdown overwrites rules.json for workers #324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lorak89 opened this issue Apr 7, 2025 · 2 comments
Open

Enabling graceful shutdown overwrites rules.json for workers #324

lorak89 opened this issue Apr 7, 2025 · 2 comments

Comments

@lorak89
Copy link

lorak89 commented Apr 7, 2025

I have following rules defined in helm chart to be able to read metrics from trino coordinator and workers:

    accessControl:
      type: configmap
      refreshPeriod: 60s
      configFile: "rules.json"
      rules:
        rules.json: |-
          {
            "system_information": [
              {
                "role": "admin",
                "allow": ["read", "write"]
              },
              {
                "user": "foo",
                "allow": ["read"]
              }
            ]
          }

when enabling graceful shutdown, it automatically overwrites above with

{
  "system_information": [
    {
      "allow": [
        "write"
      ],
      "user": "admin"
    }
  ]
}

hence reading metrics for foo user stops working. Looks like it happens here: https://github.com/trinodb/charts/blob/main/charts/trino/templates/deployment-worker.yaml#L75

I think it would be good to have some switch to enable/disable creation of system information access when enabling graceful shutdown.

@nineinchnick
Copy link
Member

I don't see the rules from accessControl being mounted on workers, so I don't think graceful shutdown overwrites anything. The description of accessControl doesn't mention this, either.

We have to decide how to allow configuring access control on worker nodes.

You should be able to work around it by not enabling the graceful shutdown property, but configuring it manually.

@lorak89
Copy link
Author

lorak89 commented Apr 7, 2025

Thank You for the reply, I assumed it overwrites it because I was able to access metrics endpoint as a foo user before enabling graceful shutdown. I will try with graceful shutdown manual setup for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants