Allow ECK-Operator to easily watch deployed namespace#8830
Allow ECK-Operator to easily watch deployed namespace#8830developer67890 wants to merge 3 commits intoelastic:mainfrom
Conversation
|
❌ Author of the following commits did not sign a Contributor Agreement: Please, read and sign the above mentioned agreement if you want to contribute to this project |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
| webhook-port: {{ .Values.webhook.port }} | ||
| {{- end }} | ||
| {{- with .Values.managedNamespaces }} | ||
| {{- if .Values.singleNamespaceMode }} |
There was a problem hiding this comment.
Should we validate that managedNamespaces is not set if singleNamespaceMode is true?
Something along the lines of:
| {{- if .Values.singleNamespaceMode }} | |
| {{- if and .Values.singleNamespaceMode .Values.managedNamespaces }} | |
| {{- fail "managedNamespaces should not be set when singleNamespaceMode is enabled" }} | |
| {{- end }} | |
| {{- if .Values.singleNamespaceMode }} |
barkbay
left a comment
There was a problem hiding this comment.
Let's also create some unit tests in deploy/eck-operator/templates/tests/configmap_test.yaml:
# yaml-language-server: $schema=https://raw.githubusercontent.com/helm-unittest/helm-unittest/main/schema/helm-testsuite.json
suite: test configmap
templates:
- configmap.yaml
tests:
- it: should render configmap with default values
asserts:
- isKind:
of: ConfigMap
- equal:
path: metadata.name
value: elastic-operator
- it: should set namespaces to release namespace when singleNamespaceMode is enabled
set:
singleNamespaceMode: true
release:
namespace: my-namespace
asserts:
- isKind:
of: ConfigMap
- matchRegex:
path: data.eck\.yaml
pattern: "namespaces: \\[my-namespace\\]"
- it: should set namespaces from managedNamespaces when singleNamespaceMode is disabled
set:
singleNamespaceMode: false
managedNamespaces:
- ns1
- ns2
- ns3
asserts:
- isKind:
of: ConfigMap
- matchRegex:
path: data.eck\.yaml
pattern: "namespaces: \\[ns1,ns2,ns3\\]"
- it: should fail when both singleNamespaceMode and managedNamespaces are set
set:
singleNamespaceMode: true
managedNamespaces:
- ns1
asserts:
- failedTemplate:
errorMessage: "managedNamespaces should not be set when singleNamespaceMode is enabled"
- it: should not set namespaces when neither singleNamespaceMode nor managedNamespaces are set
set:
singleNamespaceMode: false
managedNamespaces: []
asserts:
- isKind:
of: ConfigMap
- notMatchRegex:
path: data.eck\.yaml
pattern: "namespaces:"
- it: should set metrics port correctly
set:
config:
metrics:
port: 9090
secureMode:
enabled: false
asserts:
- isKind:
of: ConfigMap
- matchRegex:
path: data.eck\.yaml
pattern: "metrics-port: 9090"
- it: should fail when metrics secureMode is enabled but port is 0
set:
config:
metrics:
port: 0
secureMode:
enabled: true
asserts:
- failedTemplate:
errorMessage: "config.metrics.port must be greater than 0 when config.metrics.secureMode.enabled is true"
- it: should set container registry correctly
set:
config:
containerRegistry: my-registry.example.com
asserts:
- isKind:
of: ConfigMap
- matchRegex:
path: data.eck\.yaml
pattern: "container-registry: my-registry.example.com"
- it: should set setDefaultSecurityContext correctly
set:
config:
setDefaultSecurityContext: "true"
asserts:
- isKind:
of: ConfigMap
- matchRegex:
path: data.eck\.yaml
pattern: "set-default-security-context: true"
barkbay
left a comment
There was a problem hiding this comment.
@developer67890 sorry for the very late review, I think I'm 👍 with your proposal. I let a comment about adding a validation and some unit tests. Let me know what you think.
This change adds a Helm Chart value
singleNamespaceModewhich configures to the operator to watch its deployed namespace if true.While this could have been achieved by including the deployed namespace in the
managedNamespaceslist, it requires a user to specify a second time this namespace.This model is fragile and prone to mistyping or omissions. Other products have accomplished a similar feature by including something like
clusterWide=false, which will result in the same behavior this PR achieves.Changes:
singleNamespaceModevalue tovalues.yaml, including short comment explanationsingleNamespaceMode = true, only include.Release.Namespacein thenamespacesfield of the Configmap.Resolves #8828