This repository was archived by the owner on Jun 18, 2025. It is now read-only.
Draft
Conversation
Default value is false to maintain backward compatability
Updated logic around RoleBinding vs ClusterRoleBinding based on watchAllNamespaces value
|
|
jtyr
reviewed
Mar 30, 2022
|
|
||
|
|
||
|
|
||
| ### Helm Chart |
|
|
||
| The Helm chart consists of several components. The Kubernetes configurations associated with the Helm chart are located under `crds/` and `templates/`. | ||
|
|
||
| #### Custom Resource Definition |
|
|
||
| The Custom Resource Definition under `crds/app.terraform.io_workspaces_crd.yaml` defines that the Workspace Custom Resource schema. | ||
|
|
||
| #### Role-Based Access Control |
| fieldPath: metadata.namespace | ||
| ``` | ||
|
|
||
| When deploying, if you want to explicitly watch all namespaces, |
There was a problem hiding this comment.
Line wrapping should be on 80 characters.
|
|
||
| #### Namespace Scope | ||
|
|
||
| To ensure the operator does not have access to secrets or resource beyond the namespace, the Helm chart scopes the operator's deployment to a namespace. |
There was a problem hiding this comment.
Line wrapping should be on 80 characters.
| Define the kind of Role to use | ||
| */}} | ||
| {{- define "terraform.getRole" -}} | ||
| {{- if .Values.syncWorkspace.watchAllNamespaces -}} |
There was a problem hiding this comment.
I would change this to this:
{{/*
Define the kind of Role to use
*/}}
{{- define "terraform.getRole" -}}
{{- if or .Values.syncWorkspace.watchAllNamespaces (or (empty .Values.syncWorkspace.k8WatchNamespace) (eq (.Values.syncWorkspace.k8WatchNamespace | toString) .Release.Namespace)) }}
{{- "ClusterRole" }}
{{- else }}
{{- "Role" }}
{{- end }}
{{- end -}}
|
I would love to see this PR merged - any chance of getting some traction here please ? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The motivation for this PR is that currently, this helm chart only provides the option to watch a single namespace either be it the release namespace or the value in the
k8WatchNamespace. This PR provides the functionality to explicitly allow the operator to watch all namespaces.This pull request adds:
watchAllNamespaces(default tofalse) to watch for workspace CRDs in all namespacesk8WatchNamespacevar and the newwatchAllNamespacesvarWe're raising this as a draft PR and then If you folks are OK with these changes - we can add all the required tests as per guidelines.