Skip to content
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

Add service account template to graphql-engine #19

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

seth-acuitymd
Copy link
Contributor

Adds a service account template for the graphql-engine chart. This template will be rendered (default name: graphql-engine) when serviceAccount.enabled is set to true, also allows adding desired annotations or labels to the service account.

Fixes #18

@seth-acuitymd
Copy link
Contributor Author

Just some tests with helm template .

  • setting to enabled: false - nothing happens (as expected)
  • setting to enabled: true with nothing else set:
apiVersion: v1
kind: ServiceAccount
metadata:
  name: graphql-engine
  namespace: default

  • setting to enabled: true with a label and annotation:
# values
serviceAccount:
  enabled: true
  name: ""
  annotations:
    test: "test-1"
    test2: "test-2"
  labels: 
    test-label: "test-label"

# helm template output
apiVersion: v1
kind: ServiceAccount
metadata:
  name: graphql-engine
  namespace: default
  annotations:
    test: test-1
    test2: test-2
  labels:
    test-label: test-label

  • setting to enabled: true just a custom service account name:
# values
serviceAccount:
  enabled: true
  name: "thisisatestserviceaccountname"
  annotations: {}
  labels: {}

# helm template output
apiVersion: v1
kind: ServiceAccount
metadata:
  name: thisisatestserviceaccountname
  namespace: default

@@ -96,6 +96,8 @@ healthChecks:
serviceAccount:
enabled: false
name: ""
Copy link
Collaborator

@hgiasac hgiasac Apr 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we just set graphql-engine directly to this name field. Users won't need to explore the helm source template to know the default service account name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works! I wasn't sure if there was a preference!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hgiasac was that a request or suggestion? Shall I push that as a change?

@hgiasac hgiasac merged commit cab909c into hasura:main Apr 10, 2024
1 check passed
@hgiasac
Copy link
Collaborator

hgiasac commented Apr 10, 2024

LGTM. Thanks for contributing

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

Successfully merging this pull request may close these issues.

Setting serviceAccount.enabled: true does not create a service account
2 participants