-
Notifications
You must be signed in to change notification settings - Fork 17
CLOUDP-322487 - Change webhook's CR and CRB resource names to include operator name and namespace #393
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
base: master
Are you sure you want to change the base?
CLOUDP-322487 - Change webhook's CR and CRB resource names to include operator name and namespace #393
Changes from 7 commits
be2afb0
dcc2ec2
c51782a
b13fdd0
d7ec9ba
26431a5
033d782
6a322f3
75cdd52
96c2679
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,13 @@ | ||
|
||
{{/* This cluster role and binding is necessary to allow the operator to automatically register ValidatingWebhookConfiguration. */}} | ||
{{- if and .Values.operator.webhook.registerConfiguration .Values.operator.webhook.installClusterRole }} | ||
{{- if not (lookup "rbac.authorization.k8s.io/v1" "ClusterRole" "" "mongodb-kubernetes-operator-mongodb-webhook") }} | ||
{{- $webhookClusterRoleName := printf "%s-%s-webhook-cr" .Values.operator.name (include "mongodb-kubernetes-operator.namespace" .) }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i've changed both names, because one was dynamic and one not - causing upgrade problems. Now both are just a new set of names. |
||
{{- $webhookClusterRoleBindingName := printf "%s-%s-webhook-crb" .Values.operator.name (include "mongodb-kubernetes-operator.namespace" .) }} | ||
--- | ||
kind: ClusterRole | ||
apiVersion: rbac.authorization.k8s.io/v1 | ||
metadata: | ||
name: {{.Values.operator.baseName}}-operator-mongodb-webhook | ||
name: {{ $webhookClusterRoleName }} | ||
rules: | ||
- apiGroups: | ||
- "admissionregistration.k8s.io" | ||
|
@@ -28,17 +29,16 @@ rules: | |
- create | ||
- update | ||
- delete | ||
{{- end }} | ||
--- | ||
|
||
kind: ClusterRoleBinding | ||
apiVersion: rbac.authorization.k8s.io/v1 | ||
metadata: | ||
name: {{ .Values.operator.name }}-{{ include "mongodb-kubernetes-operator.namespace" . }}-webhook-binding | ||
name: {{ $webhookClusterRoleBindingName }} | ||
roleRef: | ||
apiGroup: rbac.authorization.k8s.io | ||
kind: ClusterRole | ||
name: {{.Values.operator.baseName}}-operator-mongodb-webhook | ||
name: {{ $webhookClusterRoleName }} | ||
subjects: | ||
- kind: ServiceAccount | ||
name: {{ .Values.operator.name }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this test verify that subsequent upgrades of the chart does not break the rbac? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no - we only have e2e tests that verify latest (1.2.0) -> current (code build) and those are passing. We don't have current -> current, but I don't see how this could be failing, do you think we should test this in particular? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally we should have a test in our CI that upgrades the released helm chart version to the current local version to test things. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
isn't that exactly that - or are we talking about different things? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. its mongodb-kubernetes/docker/mongodb-kubernetes-tests/tests/upgrades/operator_upgrade_ops_manager.py Line 148 in f29ac63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if thats the case then what @lucian-tosa said, is already covered right? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
suite: test webhook consistent clusterrole and binding | ||
templates: | ||
- operator-roles-webhook.yaml | ||
tests: | ||
- it: should have consistent ClusterRole and ClusterRoleBinding names | ||
set: | ||
operator.webhook.registerConfiguration: true | ||
operator.webhook.installClusterRole: true | ||
asserts: | ||
- hasDocuments: | ||
count: 2 | ||
- isKind: | ||
of: ClusterRole | ||
documentIndex: 0 | ||
- isKind: | ||
of: ClusterRoleBinding | ||
documentIndex: 1 | ||
- equal: | ||
path: metadata.name | ||
value: mongodb-kubernetes-operator-NAMESPACE-webhook-cr | ||
documentIndex: 0 | ||
- equal: | ||
path: metadata.name | ||
value: mongodb-kubernetes-operator-NAMESPACE-webhook-crb | ||
documentIndex: 1 | ||
- equal: | ||
path: roleRef.name | ||
value: mongodb-kubernetes-operator-NAMESPACE-webhook-cr | ||
documentIndex: 1 | ||
|
||
# Test that different installations get unique names (prevents conflicts) | ||
- it: should create unique names per installation | ||
set: | ||
operator.name: my-operator | ||
operator.namespace: custom-ns | ||
operator.webhook.registerConfiguration: true | ||
operator.webhook.installClusterRole: true | ||
release: | ||
namespace: custom-ns | ||
asserts: | ||
- equal: | ||
path: metadata.name | ||
value: my-operator-custom-ns-webhook-cr | ||
documentIndex: 0 | ||
- equal: | ||
path: metadata.name | ||
value: my-operator-custom-ns-webhook-crb | ||
documentIndex: 1 | ||
- equal: | ||
path: roleRef.name | ||
value: my-operator-custom-ns-webhook-cr | ||
documentIndex: 1 |
Uh oh!
There was an error while loading. Please reload this page.