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

helm chart: Add extra labels and annotations to pods and k8s resources / Allow SSL mode to be defined. #655

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion charts/nebraska/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ sources:
maintainers:
- name: kinvolk
url: https://kinvolk.io/
version: 1.1.0
version: 1.1.1
Copy link
Contributor

Choose a reason for hiding this comment

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

Please bump at least the minor version as this PR adds additional features and not bugfixes.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the review, I'm not a Helm user myself and don't know how the versioning is used in practice… So rather semver-like, it seems.

appVersion: "2.8.6"

dependencies:
Expand Down
4 changes: 4 additions & 0 deletions charts/nebraska/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ $ helm install my-nebraska nebraska/nebraska
| `strategy.rollingUpdate.maxSurge` | The maximum number of pods that can be scheduled above the desired number of pods (Only applies when `strategy.type` is `RollingUpdate`) | `nil` |
| `strategy.rollingUpdate.maxUnavailable` | The maximum number of pods that can be unavailable during the update (Only applies when `strategy.type` is `RollingUpdate`) | `nil` |
| `podAnnotations` | Annotations for pods | `nil` |
| `podLabels` | Labels for pods | `nil` |
| `extraAnnotations` | Extra annotations added to all k8s resources | `nil` |
| `extraLabels` | Extra labels added to all k8s resources | `nil` |
| `podSecurityContext` | Holds pod-level security attributes and common container settings | Check `values.yaml` file |
| `securityContext` | Security options the container should run with | `nil` |
| `service.type` | Kubernetes Service type | `ClusterIP` |
Expand Down Expand Up @@ -103,6 +106,7 @@ $ helm install my-nebraska nebraska/nebraska
| `config.database.username` | PostgreSQL user | `{{ .Values.postgresql.postgresqlUsername }}` (evaluated as a template) |
| `config.database.password` | PostgreSQL user password | `""` (evaluated as a template) |
| `config.database.passwordExistingSecret.enabled` | Enables setting PostgreSQL user password via an existing secret | `true` |
| `config.database.sslMode | Use SSL for database connection | `disable` |
| `config.database.passwordExistingSecret.name` | Name of the existing secret | `{{ .Release.Name }}-postgresql` (evaluated as a template) |
| `config.database.passwordExistingSecret.key` | Key inside the existing secret containing the PostgreSQL user password | `postgres-password` |
| `extraArgs` | Extra arguments to pass to Nebraska binary | `[]` |
Expand Down
11 changes: 10 additions & 1 deletion charts/nebraska/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,20 @@
{{- $host := .Values.config.database.host | default (include "nebraska.postgresql.fullname" .) }}
{{- $port := .Values.config.database.port | toString }}
{{- $user := ( tpl .Values.config.database.username . ) }}
{{- $sslMode := ( tpl .Values.config.database.sslMode . ) }}
apiVersion: apps/v1
kind: Deployment
metadata:
name: {{ include "nebraska.fullname" . }}
labels:
{{- include "nebraska.labels" . | nindent 4 }}
{{- with .Values.extraLabels }}
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with.Values.extraAnnotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
spec:
replicas: {{ .Values.replicaCount }}
revisionHistoryLimit: {{ .Values.revisionHistoryLimit }}
Expand All @@ -24,6 +32,7 @@ spec:
{{- end }}
labels:
{{- include "nebraska.selectorLabels" . | nindent 8 }}
{{ toYaml .Values.podLabels | nindent 8 }}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like adding extra pod labels in this way does not work.

Suggested change
{{ toYaml .Values.podLabels | nindent 8 }}
{{- with .Values.podLabels }}
{{ toYaml . | nindent 8 }}
{{ - end }}

should fix the linter issue.

spec:
{{- with .Values.image.pullSecrets }}
imagePullSecrets:
Expand Down Expand Up @@ -137,7 +146,7 @@ spec:
key: dbPassword
{{- end }}
- name: NEBRASKA_DB_URL
value: {{ printf "postgres://%s:$(DB_PASSWORD)@%s:%s/%s?sslmode=disable&connect_timeout=10" $user $host $port $db | quote }}
value: {{ printf "postgres://%s:$(DB_PASSWORD)@%s:%s/%s?sslmode=%s&connect_timeout=10" $user $host $port $db $sslMode | quote }}
{{- if eq .Values.config.auth.mode "github" }}
- name: "NEBRASKA_GITHUB_OAUTH_CLIENT_ID"
value: "{{ .Values.config.auth.github.clientID }}"
Expand Down
6 changes: 6 additions & 0 deletions charts/nebraska/templates/ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,16 @@ metadata:
name: {{ $fullName }}
labels:
{{- include "nebraska.labels" . | nindent 4 }}
{{- with .Values.extraLabels }}
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with .Values.ingress.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with.Values.extraAnnotations }}
{{- toYaml . | nindent 4 }}
{{- end }}
Comment on lines 13 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

This block needs more attention.

If ingress.annotations is not set and you set extraAnnotations, the manifest is no longer valid as the annotations: line is not rendered then.

spec:
{{- if eq (include "nebraska.ingress.apiVersion" $) "networking.k8s.io/v1" }}
{{- with .Values.ingress.ingressClassName }}
Expand Down
9 changes: 9 additions & 0 deletions charts/nebraska/templates/pvc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ metadata:
name: {{ include "nebraska.fullname" . }}-packages
labels:
{{- include "nebraska.labels" . | nindent 4 }}
{{ toYaml .Values.config.hostFlatcarPackages.persistence.labels | nindent 4 }}
{{- with .Values.extraLabels }}
{{- toYaml . | nindent 4 }}
{{- end }}
annotations:
Copy link
Contributor

Choose a reason for hiding this comment

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

Almost same here, please surround the annotations: line with an if conditional or use a sprig merge function to merge both anntations slices together.

{{ toYaml .Values.config.hostFlatcarPackages.persistence.annotations | nindent 4 }}
{{- with.Values.extraAnnotations }}
{{- toYaml . | nindent 4 }}
{{- end }}
spec:
accessModes: {{ .Values.config.hostFlatcarPackages.persistence.accessModes }}
resources:
Expand Down
7 changes: 7 additions & 0 deletions charts/nebraska/templates/secrets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ metadata:
name: {{ include "nebraska.fullname" . }}
labels:
{{- include "nebraska.labels" . | nindent 4 }}
{{- with .Values.extraLabels }}
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with.Values.extraAnnotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
type: Opaque
data:
{{- if $useDbPassword }}
Expand Down
7 changes: 7 additions & 0 deletions charts/nebraska/templates/service.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ metadata:
name: {{ include "nebraska.fullname" . }}
labels:
{{- include "nebraska.labels" . | nindent 4 }}
{{- with .Values.extraLabels }}
{{- toYaml . | nindent 4 }}
{{- end }}
annotations:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

{{- with.Values.extraAnnotations }}
{{- toYaml . | nindent 4 }}
{{- end }}
spec:
type: {{ .Values.service.type }}
ports:
Expand Down
6 changes: 6 additions & 0 deletions charts/nebraska/templates/serviceaccount.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@ metadata:
name: {{ include "nebraska.serviceAccountName" . }}
labels:
{{- include "nebraska.labels" . | nindent 4 }}
{{- with .Values.extraLabels }}
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with .Values.serviceAccount.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with.Values.extraAnnotations }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

{{- toYaml | nindent 4 }}
{{- end }}
{{- end }}
7 changes: 7 additions & 0 deletions charts/nebraska/templates/update-ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@ metadata:
name: {{ $fullName | trunc 56 | trimSuffix "-" }}-update
labels:
{{- include "nebraska.labels" . | nindent 4 }}
{{- with .Values.extraLabels }}
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with .Values.ingress.update.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
{{- with.Values.extraAnnotations }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

{{- toYaml . | nindent 4 }}
{{- end }}
{{ toYaml .Values.extraAnnotations | nindent 4 }}
spec:
{{- if eq (include "nebraska.ingress.apiVersion" $) "networking.k8s.io/v1" }}
{{- with .Values.ingress.update.ingressClassName }}
Expand Down
9 changes: 9 additions & 0 deletions charts/nebraska/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ config:
packagesPath: /mnt/packages
# nebraskaURL: http://flatcar.example.com
persistence:
annotations: {}
labels: {}
enabled: false
storageClass:
accessModes:
Expand Down Expand Up @@ -73,6 +75,7 @@ config:
dbname: '{{ .Values.postgresql.auth.database }}'
username: '{{ .Values.postgresql.auth.username }}'
password: ""
sslMode: disable
passwordExistingSecret:
enabled: true
name: '{{ .Release.Name }}-postgresql'
Expand Down Expand Up @@ -119,7 +122,13 @@ strategy:
# maxSurge: 25%
# maxUnavailable: 25%

# Extra labels and annotations to be set to pods
podAnnotations: {}
podLabels: {}

# Extra labels and annotations to be added to ALL resources
extraLabels: {}
extraAnnotations: {}

podSecurityContext:
runAsUser: 65534
Expand Down
Loading