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

Working minimal authn/authz configuration for blueapi #284

Closed
wants to merge 4 commits into from
Closed
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
5 changes: 5 additions & 0 deletions helm/blueapi/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,8 @@ type: application
# Versions are expected to follow Semantic Versioning (https://semver.org/)
# This is updated automatically by CI, the appVersion is automatically inserted by CI
version: 0.1.0

dependencies:
- name: rabbitmq
version: 11.16.2
repository: oci://registry-1.docker.io/bitnamicharts
Comment on lines +21 to +24
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
dependencies:
- name: rabbitmq
version: 11.16.2
repository: oci://registry-1.docker.io/bitnamicharts
dependencies:
- name: rabbitmq
version: 11.16.2
repository: oci://registry-1.docker.io/bitnamicharts
enabled: rabbitmq.enabled

If we add this to the conditions, we can also add
rabbitmq:
enabled: false

to our values.yaml to disable rabbitmq (once we have BlueAPI not requiring a message bus)

10 changes: 10 additions & 0 deletions helm/blueapi/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ app.kubernetes.io/name: {{ include "blueapi.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
{{- end }}

{{/*
Metadata labels
*/}}
{{- define "blueapi.metadataLabels" -}}
{{ include "blueapi.selectorLabels" . }}
{{- range $key, $value := .Values.extraLabels }}
{{- $key }}: {{ $value | quote }} # N.B. ensures your labels are correctly String->String
{{- end }}
{{- end }}

Comment on lines +53 to +62
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{{/*
Metadata labels
*/}}
{{- define "blueapi.metadataLabels" -}}
{{ include "blueapi.selectorLabels" . }}
{{- range $key, $value := .Values.extraLabels }}
{{- $key }}: {{ $value | quote }} # N.B. ensures your labels are correctly String->String
{{- end }}
{{- end }}
{{/*
Metadata labels
*/}}
{{- define "blueapi.extraLabels" -}}
{{- range $key, $value := .Values.extraLabels }}
{{- $key }}: {{ $value | quote }} # N.B. ensures your labels are correctly String->String
{{- end }}

{{/*
Create the name of the service account to use
*/}}
Expand Down
1 change: 0 additions & 1 deletion helm/blueapi/templates/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,3 @@ metadata:
data:
config.yaml: |-
{{- toYaml .Values.worker | nindent 4 }}

8 changes: 7 additions & 1 deletion helm/blueapi/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ spec:
{{- toYaml . | nindent 8 }}
{{- end }}
labels:
{{- include "blueapi.selectorLabels" . | nindent 8 }}
{{- include "blueapi.metadataLabels" . | nindent 8 }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

{{- include "blueapi.selectorLabels" . | nindent 8 }}
{{- include "blueapi.extraLabels . | nindent 8 }}

I think it'd make more sense to present the extraLabels alongside the selectorLabels, rather than folding both into a single function: there are selectorLabels which are used by this release to map between resources and extraLabels, which have some meaning in a wider context.

spec:
{{- with .Values.imagePullSecrets }}
imagePullSecrets:
Expand All @@ -35,6 +35,9 @@ spec:
- secret:
name: {{ . }}
{{- end }}
{{- with .Values.extraVolumes -}}
{{ toYaml . | nindent 6 }}
{{- end }}
containers:
- name: {{ .Chart.Name }}
securityContext:
Expand All @@ -56,6 +59,9 @@ spec:
- "/config/secret.yaml"
{{- end }}
- "serve"
{{- with .Values.extraContainers }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
Expand Down
36 changes: 32 additions & 4 deletions helm/blueapi/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,41 @@ worker:
env:
sources:
- kind: deviceFunctions
module: blueapi.startup.example
module: blueapi.startup.example_devices
- kind: planFunctions
module: blueapi.startup.example_plans
- kind: planFunctions
module: blueapi.plans
stomp:
auth:
username: guest
passcode: guest
host: rabbitmq
username: foo
passcode: bar
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm going to make a ticket for describing how to make SealedSecret for BlueAPI and RabbitMQ shared information, I think for this default/example config, we can keep guest/guest and add to the readme for deploying that these values will need to be overwritten if rabbitmq.enabled and not exposed on localhost

Comment on lines +85 to +86
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
username: foo
passcode: bar
username: guest
passcode: guest

host: localhost
port: 61613
# Config for the worker goes here, will be mounted into a config file

extraLabels: {}
extraVolumes: {}
extraContainers: {}

rabbitmq:
persistence:
enabled: true # TODO: Best approach for storage
auth:
username: foo
password: bar
Comment on lines +99 to +100
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
username: foo
password: bar
username: guest
password: guest

extraPlugins: rabbitmq_stomp
rbac:
create: false
serviceAccount:
name: default-full-access-mounted # TODO: use only required permissions
create: false
extraContainerPorts:
- name: stomp
containerPort: 61613
service:
extraPorts:
- name: stomp
port: 61613
targetPort: stomp
nodePort: null