feat: add disablesuffixes option for backwards compat

[aarlaud]

Adding --set disableSuffixes=true value will avoid adding suffixes to objects, reverting to old behavior where one can have only one broker per namespace, mainly for backward compatibility.

[ahmed-agabani-snyk]

General practice:

• I do not recommend changing names using boolean flags (disableSuffixes=true).
• I recommend allowing the consumer to specify the name of the resource using ./helm/values.yaml

Suggestion:

• consolidate the if statements into the _helpers.tpl and use {{ include "snyk-broker.************Name" . }} in templates.

Example demonstrating general practice and suggestion:

# ./helm/templates/_helpers.tpl

{{/*
Expand the name of the chart.
*/}}
{{- define "tor-operator.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
{{- end }}

{{/*
Create a default fully qualified app name.
We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
If release name contains chart name it will be used as a full name.
*/}}
{{- define "snyk-broker.fullname" -}}
{{- if .Values.fullnameOverride }}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- $name := default .Chart.Name .Values.nameOverride }}
{{- if contains $name .Release.Name }}
{{- .Release.Name | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
{{- end }}
{{- end }}
{{- end }}

{{/*
Create the name of the broker deployment to use
*/}}
{{- define "snyk-broker.brokerDeploymentName" -}}
{{- if .Values.brokerDeployment.create }}
{{- default (include "snyk-broker.fullname" .) .Values.brokerDeployment.name }}
{{- else }}
{{- default "default" .Values.brokerDeployment.name }}
{{- end }}
{{- end }}

{{/*
Create the name of the service account to use
*/}}
{{- define "snyk-broker.serviceAccountName" -}}
{{- if .Values.serviceAccount.create }}
{{- default (include "snyk-broker.fullname" .) .Values.serviceAccount.name }}
{{- else }}
{{- default "default" .Values.serviceAccount.name }}
{{- end }}
{{- end }}

# ./helm/values.yaml

nameOverride: ""
fullnameOverride: ""

brokerDeployment:
  # Specifies whether a broker deployment should be created
  create: true
  # Annotations to add to the broker deployment
  annotations: {}
  # The name of the broker deployment to use.
  # If not set and create is true, a name is generated using the fullname template
  name: ""

serviceAccount:
  # Specifies whether a service account should be created
  create: 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
  name: ""

# ./helm/templates/brokerdeployment.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: {{ include "snyk-broker.brokerDeploymentName" . }}
  {{- with .Values.brokerDeployment.annotations }}
  annotations:
    {{- toYaml . | nindent 4 }}
  {{- end }}
spec:
  template:
    metadata:
    spec:
      serviceAccountName: {{ include "snyk-broker.serviceAccountName" . }}

[aarlaud]

General practice:

• I do not recommend changing names using boolean flags (disableSuffixes=true).
• I recommend allowing the consumer to specify the name of the resource using ./helm/values.yaml

Suggestion:

• consolidate the if statements into the _helpers.tpl and use {{ include "snyk-broker.************Name" . }} in templates.

Example demonstrating general practice and suggestion:

# ./helm/templates/_helpers.tpl

{{/*
Expand the name of the chart.
*/}}
{{- define "tor-operator.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
{{- end }}

{{/*
Create a default fully qualified app name.
We truncate at 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec).
If release name contains chart name it will be used as a full name.
*/}}
{{- define "snyk-broker.fullname" -}}
{{- if .Values.fullnameOverride }}
{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- $name := default .Chart.Name .Values.nameOverride }}
{{- if contains $name .Release.Name }}
{{- .Release.Name | trunc 63 | trimSuffix "-" }}
{{- else }}
{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" }}
{{- end }}
{{- end }}
{{- end }}

{{/*
Create the name of the broker deployment to use
*/}}
{{- define "snyk-broker.brokerDeploymentName" -}}
{{- if .Values.brokerDeployment.create }}
{{- default (include "snyk-broker.fullname" .) .Values.brokerDeployment.name }}
{{- else }}
{{- default "default" .Values.brokerDeployment.name }}
{{- end }}
{{- end }}

{{/*
Create the name of the service account to use
*/}}
{{- define "snyk-broker.serviceAccountName" -}}
{{- if .Values.serviceAccount.create }}
{{- default (include "snyk-broker.fullname" .) .Values.serviceAccount.name }}
{{- else }}
{{- default "default" .Values.serviceAccount.name }}
{{- end }}
{{- end }}

# ./helm/values.yaml

nameOverride: ""
fullnameOverride: ""

brokerDeployment:
  # Specifies whether a broker deployment should be created
  create: true
  # Annotations to add to the broker deployment
  annotations: {}
  # The name of the broker deployment to use.
  # If not set and create is true, a name is generated using the fullname template
  name: ""

serviceAccount:
  # Specifies whether a service account should be created
  create: 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
  name: ""

# ./helm/templates/brokerdeployment.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: {{ include "snyk-broker.brokerDeploymentName" . }}
  {{- with .Values.brokerDeployment.annotations }}
  annotations:
    {{- toYaml . | nindent 4 }}
  {{- end }}
spec:
  template:
    metadata:
    spec:
      serviceAccountName: {{ include "snyk-broker.serviceAccountName" . }}

Agreed on the recommendations and best practices. Will merge this in for now and will look at refactoring this the suggested way at a later time.