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

Dynamic namespace support #3759

Merged
merged 11 commits into from
Jan 2, 2025
Merged

Conversation

thallgren
Copy link
Member

@thallgren thallgren commented Dec 29, 2024

This PR changes how the Telepresence traffic-manager and mutating webhook deals with namespaces so that everything is controlled by one single namespace selector.

The top level Helm chart value namespaceSelector was added. It is a verbatim Kubernetes namespace selector, and deprecates the agentInjector.webhook.namespaceSelector and managerRbac.namespaces. It also acts as the default for clientRbac.namespaces.

The short form value namespaces was also added. It is a list of names and mutually exclusive to namespaceSelector. The list will get transformed into:

namespaceSelector:
  matchExpressions:
  - key: kubernetes.io/metadata.name
    operator: In
    values: [ <namespaces> ]

A selector can be dynamic or static. This in turn controls if telepresence is "cluster-wide" or "namespaced". A dynamic selector
requires cluster-wide permissions for the traffic-manager, and only a static selector can serve as base when installing Role/RoleBinding pairs.

A selector is considered static if it meets the following conditions:

  • The selector must have exactly one element in the matchLabels or the
    matchExpression list (if the element is in the matchLabels list,
    it is normalized into "key in [value]").
  • The element must meet the following criteria:
    • The key of the element must be "kubernetes.io/metadata.name".
    • The operator of the element must be "In" (case-sensitive).
    • The values list of the element must contain at least one value.

@thallgren thallgren linked an issue Dec 29, 2024 that may be closed by this pull request
@thallgren thallgren added the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 29, 2024
@thallgren thallgren requested a review from P0lip December 29, 2024 10:40
@github-actions github-actions bot removed the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 29, 2024
@thallgren thallgren added the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 29, 2024
@github-actions github-actions bot removed the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 29, 2024
@thallgren thallgren force-pushed the thallgren/namespaces-and-rbac branch from 0f7db00 to 0731a0d Compare December 29, 2024 15:20
@thallgren thallgren added the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 29, 2024
@github-actions github-actions bot removed the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 29, 2024
@thallgren thallgren added the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 29, 2024
@github-actions github-actions bot removed the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 29, 2024
@thallgren thallgren force-pushed the thallgren/namespaces-and-rbac branch 2 times, most recently from 803e9cb to 80e5bc8 Compare December 30, 2024 07:23
This commit changes how the Telepresence traffic-manager and mutating
webhook deals with namespaces so that everything is controlled by one
single namespace selector.

A selector can be _dynamic_ or _static_. This in turn controls if
telepresence is "cluster-wide" or "namespaced". A _dynamic_ selector
requires cluster-wide permissions for the traffic-manager, and only a
_static_ selector can serve as base when installing Role/RoleBinding
pairs.

A selector is considered static if it meets the following conditions:
- The selector must not have `matchLabels`.
- The selector must have exactly one element in the `matchLabels` or the
  `matchExpression` list (if the element is in the `matchLabels` list,
   it is normalized into "key in [value]").
- The element must meet the following criteria:
  - The `key` of the element must be "kubernetes.io/metadata.name".
  - The `operator` of the element must be "In" (case-sensitive).
  - The `values` list of the element must contain at least one value.

Signed-off-by: Thomas Hallgren <thomas@tada.se>
Signed-off-by: Thomas Hallgren <thomas@tada.se>
The current order is very incosistent and hard to follow. This commit
sorts the list alphabetically, without exceptions.

Signed-off-by: Thomas Hallgren <thomas@tada.se>
Add description of new `namespaces` and `namespaceSelector` values, and
deprecate the `namespaced` and `namespaces` in `client.Rbac` and
`manager.Rbac`.

Signed-off-by: Thomas Hallgren <thomas@tada.se>
Signed-off-by: Thomas Hallgren <thomas@tada.se>
Signed-off-by: Thomas Hallgren <thomas@tada.se>
Signed-off-by: Thomas Hallgren <thomas@tada.se>
The traffic-manager's IP is used for DNS in the client, so it must be
included in when computing the pod subnets. It must be added explicitly
now, because it's no longer required that the traffic-manager's own
namespace is managed.

Signed-off-by: Thomas Hallgren <thomas@tada.se>
@thallgren thallgren force-pushed the thallgren/namespaces-and-rbac branch from 80e5bc8 to a9540d9 Compare December 30, 2024 07:25
@thallgren thallgren added the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 30, 2024
@github-actions github-actions bot removed the ok to test Applied by maintainers when a PR is ready to have tests run on it label Dec 30, 2024
Copy link
Contributor

@P0lip P0lip left a comment

Choose a reason for hiding this comment

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

LGTM. I like this change.

@@ -60,7 +54,7 @@ rules:
- list
- get
- watch
{{- if .Values.agentInjector.enabled }}
{{- if .agentInjector.enabled }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's an older code, but I believe this could be merged with the above. We'd only need to add create here and the previous definition could be dropped.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I don't think that's correct. This entry is specifically for the configmap named "telepresence-agents". The entry above isn't (using create isn't possible for a named resource).

cmd/traffic/cmd/manager/cluster/info.go Outdated Show resolved Hide resolved
cmd/traffic/cmd/manager/config/config.go Show resolved Hide resolved
cmd/traffic/cmd/manager/namespaces/watcher.go Outdated Show resolved Hide resolved
thallgren and others added 2 commits January 2, 2025 15:19
Co-authored-by: Jakub Rożek <jrozek@datawire.io>
Signed-off-by: Thomas Hallgren <thomas@tada.se>
- Make k8sapi.CanI a variadic function (and use where applicable).
- Sort AgentEnv.Excluded before checking slices for equality.

Signed-off-by: Thomas Hallgren <thomas@tada.se>
@thallgren thallgren added the ok to test Applied by maintainers when a PR is ready to have tests run on it label Jan 2, 2025
@github-actions github-actions bot removed the ok to test Applied by maintainers when a PR is ready to have tests run on it label Jan 2, 2025
Signed-off-by: Thomas Hallgren <thomas@tada.se>
@thallgren thallgren added the ok to test Applied by maintainers when a PR is ready to have tests run on it label Jan 2, 2025
@github-actions github-actions bot removed the ok to test Applied by maintainers when a PR is ready to have tests run on it label Jan 2, 2025
@thallgren thallgren merged commit 13f0c83 into release/v2 Jan 2, 2025
11 checks passed
@thallgren thallgren deleted the thallgren/namespaces-and-rbac branch January 2, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Namespace selected via selector
2 participants