-
Notifications
You must be signed in to change notification settings - Fork 37
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
Remove the need to include the proxy role in superUserRoles #125
base: master
Are you sure you want to change the base?
Remove the need to include the proxy role in superUserRoles #125
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -178,7 +178,7 @@ spec: | |||
- name: ClusterName | |||
value: "{{ template "pulsar.fullname" . }}" | |||
- name: SuperRoles | |||
value: {{ .Values.superUserRoles }} | |||
value: "{{ .Values.superUserRoles }},{{ .Values.proxyRoles }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this SuperRoles
env var do here? I see it in other templates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That what Burnell calls the config option for the roles. https://github.com/datastax/burnell/blob/5a7c261e498ff5b34356b0c164d357e9f3a8b81b/src/workflow/keys-jwt.go#L320
I'm starting to doubt this change. In the Pulsar docs it says I'd assume that as long as the proxy has been authorized to pass on the client's principal, the broker would trust that information for doing the authentication decision. Why would all resources need to contain the proxy role? |
Fixes datastax#125 ### Motivation The default images in the values.yaml are in docker hub. This PR allows us to provide image pull secrets for the containers which will allow us to get around Docker Hub's rate limiting if the nodes are not logged into Docker Hub. ### Modifications Added a new template to generate `imagePullSecrets`, and included them in the deployments and statefulsets. This will only add them if they are specified under `images.imagePullSecrets` ### Verifying this change - [] Make sure that the change passes the CI checks.
Fixes #124