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

support namespace labeling for tenant owners #407

Closed
MaxFedotov opened this issue Aug 26, 2021 · 14 comments · Fixed by #423
Closed

support namespace labeling for tenant owners #407

MaxFedotov opened this issue Aug 26, 2021 · 14 comments · Fixed by #423
Assignees
Labels
enhancement New feature or request high-priority Feature Request with high-priority
Milestone

Comments

@MaxFedotov
Copy link
Collaborator

Current capsule and capsule-proxy implementations deny namespace labeling for tenant owners. The background for this decision was that there are a number of labels, which can lead to different security issues when they are added to a namespace (for example, they can allow to bypass Network Policies).

While this is true, sometimes tenant owners need to have and ability to add some well-known labels defined by their environment. For example, this can be different integrations with 3rd party systems (monitoring, configuration management) where we need to match some business applications with Kubernetes namespaces using some predefined and established set of labels.

So to support these cases and not to compromise overall security I propose adding additional allowedLabels list to namespaceOptions field in tenantSpec, where cluster-administrator can provide a list of labels, that tenant owners can modify via capsule-proxy

apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: oil
spec:
  owners:
  - name: alice
    kind: User
  namespaceOptions:
    allowedLabels:
     - myorg.net/foo
     - someother.com/bar

This will require an additional webhook for namespaces, where we allow modification of only allowed labels for a namespace.

@bsctl, @prometherion WDYT?

@MaxFedotov MaxFedotov added the blocked-needs-validation Issue need triage and validation label Aug 26, 2021
@bsctl
Copy link
Member

bsctl commented Aug 26, 2021

Can consider allowedAnnotations too?

@MaxFedotov
Copy link
Collaborator Author

Can consider allowedAnnotations too?

Yes :)

@prometherion
Copy link
Member

Maybe we should use allowedUserLabels and allowedUserAnnotations, it seems more expressive to me.

@prometherion prometherion added this to the v0.1.1 milestone Aug 28, 2021
@prometherion prometherion added enhancement New feature or request and removed blocked-needs-validation Issue need triage and validation labels Aug 28, 2021
@oliverbaehler
Copy link
Collaborator

What do you guys think about adding a Regex Option? I bootstrap certain namespaces with a helm wrapper. And I don't want users to mess with my lifecycle hooks or really anything helm related. So I would need the option to deny any label starting with helm.sh/*

@MaxFedotov
Copy link
Collaborator Author

@prometherion i think we can use existing AllowedListSpec. WDYT?

@prometherion
Copy link
Member

I thought the same, @MaxFedotov!

@MaxFedotov
Copy link
Collaborator Author

Great, i can start this after my vacation (be back home at September 12th)

@awoodobvio
Copy link

Just ran into this when using capsule with argocd. ArgoCD wants to add an annotation and a label to the namespace it is creating. We are adding an "ownerReference" to the namesapce as well to allow capsule to "adopt" the namespace after argocd creates it.

Unfortunately, capsule then removes the argo annoations and labels and that causes the two controllers to start to fight over the namespace.

Having this sounds like it would allow us to whitelist the annotations and labels that argocd added and let capsule maintain them.

@prometherion prometherion added the high-priority Feature Request with high-priority label Sep 9, 2021
@prometherion
Copy link
Member

Thanks for this additional use case, @awoodsprim.

Definitely, we're going to support this feature on v1beta1 using annotations, then we're going to plan a migration for a new API version.

@MaxFedotov
Copy link
Collaborator Author

MaxFedotov commented Sep 9, 2021

@awoodsprim i think i can propose a more interesting schema with argocd (i am planning to implement in on our production after my vacation). Starting from v1beta1 capsule allows to add multiple owners for tenants, including service accounts. So it will be possible to add argocd service account as an additional owner to each tenant by default, thus allowing it to create namespaces as a usual tenant user (and it won't be required to set onwerReference manually). And don't forget to add it to userGroups in capsuleConfiguration crd.

@awoodobvio
Copy link

@MaxFedotov - With the two tenant owners, creation of a namespace can be done with annotations on the namespace? I only ask because at the moment annotations added manually thorough kubectl are stripped by capsule as per this ticket. Therefore, having argocd as a tenant owner will solve the "adoption" issue, but not the annotation / label one (I think)

@MaxFedotov
Copy link
Collaborator Author

@awoodsprim thats depends on how you create a namespace. If you create it using yaml file with ns spec - than you can add annotations/labels to it. If you are using auto-create namespace option - than you had to wait until this proposal will be implemented :)

@MaxFedotov
Copy link
Collaborator Author

MaxFedotov commented Sep 13, 2021

after discussion with @prometherion decided to add additional protectedLabels and protectedAnnotations fields, where a list of annotations\labels that can't be modified by tenant owner could be stored. This way we will support a scenario, when there is a small list of well-known labels\annotations, which can't be set or modified by a tenant admin (and all others could be added\modified by tenant-admin).
The new structure will look like:

apiVersion: capsule.clastix.io/v1beta1
kind: Tenant
metadata:
  name: oil
spec:
  owners:
  - name: alice
    kind: User
  namespaceOptions:
    userLabels:
      allowed:
        - myorg.net/foo
      allowedRegex: ^\w+-lb$
      protected:
        - protected.net/bar
    userAnnotations:
      allowed:
        - myorg.net/foo
      allowedRegex: ^\w+-lb$
      protected:
        - protected.net/bar

@MaxFedotov
Copy link
Collaborator Author

MaxFedotov commented Sep 17, 2021

After another discussion with @prometherion decided to use annotations for now (and later when planning new Capsule api version release move them to tenant spec). And as it is very hard to support both, allowed and forbidden labels and annotations we will use only forbidden.

So with the release of this feature, capsule will allow tenant owner to add any label or annotation on a namespace.

If cluster administrator want to disallow tenant owner to use some labels or annotations, he had to add one of the following annotations on a tenant:

  • capsule.clastix.io/forbidden-namespace-labels
  • capsule.clastix.io/forbidden-namespace-labels-regexp
  • capsule.clastix.io/forbidden-namespace-annotations
  • capsule.clastix.io/forbidden-namespace-annotations-regexp

When new capsule api will be release, these annotations will be moved to the NamespaceOptions:

type NamespaceOptions struct {
	//+kubebuilder:validation:Minimum=1
	// Specifies the maximum number of namespaces allowed for that Tenant. Once the namespace quota assigned to the Tenant has been reached, the Tenant owner cannot create further namespaces. Optional.
	Quota *int32 `json:"quota,omitempty"`
	// Specifies additional labels and annotations the Capsule operator places on any Namespace resource in the Tenant. Optional.
	AdditionalMetadata *AdditionalMetadataSpec `json:"additionalMetadata,omitempty"`
	// Specifies forbidden a labels the Tenant owner can't place on Namespace resources in the Tenant. Optional.
	UserLabels *ForbiddenListSpec `json:"userLabels,omitempty"`
	// Specifies forbidden annotations the Tenant owner can't place on Namespace resources in the Tenant. Optional.
	UserAnnotations *ForbiddenListSpec `json:"userAnnotations,omitempty"`
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request high-priority Feature Request with high-priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants