Skip to content

fix: set service account default creation to false#217

Open
sk-portkey wants to merge 1 commit into
mainfrom
fix/sa_default
Open

fix: set service account default creation to false#217
sk-portkey wants to merge 1 commit into
mainfrom
fix/sa_default

Conversation

@sk-portkey

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR changes the Helm chart defaults so the chart no longer creates ServiceAccounts by default, and adjusts helper templates to compute the ServiceAccount name for gateway and dataservice.

Changes:

  • Set serviceAccount.create default to false for both gateway and dataservice in values.yaml.
  • Updated templates/_helpers.tpl to enforce a non-empty ServiceAccount name when create is false (currently via fail).
  • Updated inline comments in values.yaml around ServiceAccount naming requirements.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
charts/portkey-gateway/values.yaml Changes ServiceAccount creation defaults and updates comments for naming behavior.
charts/portkey-gateway/templates/_helpers.tpl Updates helper functions that determine which ServiceAccount name is used by workloads.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 104 to 111
{{- define "portkeyenterprise.serviceAccountName" -}}
{{- if .Values.serviceAccount.create }}
{{- default (include "portkeyenterprise.fullname" .) .Values.serviceAccount.name }}
{{- else if not .Values.serviceAccount.name }}
{{- fail "serviceAccount.name must be set when serviceAccount.create is false" }}
{{- else }}
{{- default "default" .Values.serviceAccount.name }}
{{- .Values.serviceAccount.name }}
{{- end }}
Comment on lines 114 to 121
{{- define "dataservice.serviceAccountName" -}}
{{- if .Values.dataservice.serviceAccount.create -}}
{{ default (printf "%s-%s" (include "portkeyenterprise.fullname" .) .Values.dataservice.name) .Values.dataservice.serviceAccount.name | trunc 63 | trimSuffix "-" }}
{{- else if not .Values.dataservice.serviceAccount.name -}}
{{- fail "dataservice.serviceAccount.name must be set when dataservice.serviceAccount.create is false" }}
{{- else -}}
{{ default "default" .Values.dataservice.serviceAccount.name }}
{{ .Values.dataservice.serviceAccount.name | trunc 63 | trimSuffix "-" }}
{{- end -}}
Comment on lines 156 to 165
serviceAccount:
# Specifies whether a service account should be created
create: true
create: false
# Automatically mount a ServiceAccount's API credentials?
automount: true
# Annotations to add to the service account
annotations: {}
# The name of the service account to use.
# If not set and create is true, a name is generated using the fullname template
# Required when create is false. If not set and create is true, a name is generated using the fullname template
name: ""
Comment on lines 451 to 455
serviceAccount:
create: true
create: false
# Required when create is false. If not set and create is true, a name is generated using the fullname template
name: ""
labels: {}
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.

2 participants