Add ability to create custom labels for namespaces created with syncOptions CreateNamespace

[nickjj]

First up let me just say thank you for making such a wonderful tool!

Summary

At the moment we can configure ArgoCD to create a namespace for us with:

syncOptions:
    - "CreateNamespace=true"

But we can't add custom labels to that namespace.

Motivation

To achieve zero downtime deployments with the AWS Load Balancer Controller they recommend adding a specific custom label to each namespace you deploy your application into. This is documented at https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.3/deploy/pod_readiness_gate/. This feels like something you would always want to use on AWS with EKS.

They say to run: kubectl label namespace readiness elbv2.k8s.aws/pod-readiness-gate-inject=enabled

Another use case is enabling Fluent Bit on EKS with Fargate, this allows your pods to log to CloudWatch. One requirement is to create a namespace with a custom label. Details are at https://aws.amazon.com/blogs/containers/fluent-bit-for-amazon-eks-on-aws-fargate-is-here/.

In the spirit of gitops you could explicitly create a namespace.yaml file and include that as part of your Kubernetes manifest files (raw, Kustomize, Helm, etc.) and then not use Argo's CreateNamespace=true but this feels a little dirty I think? It would mean setting a custom overlay with Kustomize for every namespace (prod, staging, qa, pr, etc.), this starts to add up.

If this feature existed we could avoid creating a namespace.yaml and instead configure our ArgoCD Application to create a namespace with 0 or more custom labels.

Proposal

I'm not sure how it should be implemented. Maybe something like:

syncOptions:
  namespace:
    create: true
    labels:
      elbv2.k8s.aws/pod-readiness-gate-inject: "enabled"

It could default to labels: {} to create no labels.

What do you think? In the mean time is the explicit creation of a namespace.yaml a reasonable and gitops friendly way to do this?

[ledroide]

Similar to #6215

[ledroide]

@nickjj : If you define the namespace resource in a yaml manifest, and set a spec.syncPolicy.automated.prune=true in your argoproj.io/application resource, then there will be a namespace prune/creation loop every time you commit any change.

[lukpep]

also waiting for this feature - super handy

[rufreakde]

Thumbs up from me as well! @nickjj is this only a proposal or is there work done for this already? I think the more suitable place would be:

kind: Application  
...
spec:
...
  destination:
    server: <server>
    namespace: 
      name: <name>
      autoCreate: true
      addLabels:
        - istio-injection: "disabled" # just an example
      addAnnotations:
        - imageregistry: "https://hub.docker.com/" # just an example

I think the automatic creation of a namespace should not be a sync option it should be part of the destination definition as sync is usually a reoccurring process. But the creation of a destination is done only once? On the other hand of course we would want to keep the added labels in sync there as well if some time in the future you want to add some more to the namespace. This is similar with annotations as well.

@nickjj : If you define the namespace resource in a yaml manifest, and set a spec.syncPolicy.automated.prune=true in your argoproj.io/application resource, then there will be a namespace prune/creation loop every time you commit any change. @ledroide Does it mean it is not advised to create namespace.yaml manifest files when you need custom annotations. If you define spec.syncPolicy.automated.prune=true?

[nickjj]

@rufreakde It was a feature request for now, I haven't done any work to implement it.

[ledroide]

@rufreakde

Does it mean it is not advised to create namespace.yaml manifest files when you need custom annotations. If you define spec.syncPolicy.automated.prune=true?

If you want to experience the loop, you can try it easily. Otherwise, just don't do it.

By the way, fields like addLabels and addAnnotations in the argoproj.io/application resource looks good to me. However, usually they are defined as dictionaries, not as arrays :

    namespace: 
      addLabels:
        istio-injection: "disabled" 
      addAnnotations:
        imageregistry: "https://hub.docker.com/"

[rufreakde]

@rufreakde

Does it mean it is not advised to create namespace.yaml manifest files when you need custom annotations. If you define spec.syncPolicy.automated.prune=true?

If you want to experience the loop, you can try it easily. Otherwise, just don't do it.

By the way, fields like addLabels and addAnnotations in the argoproj.io/application resource looks good to me. However, usually they are defined as dictionaries, not as arrays :

    namespace: 
      addLabels:
        istio-injection: "disabled" 
      addAnnotations:
        imageregistry: "https://hub.docker.com/"

Thanks for the feedback. Both ways of the fields are valid to me if it is more common to not use arrays this also looks great!

[booleanbetrayal]

We are also butting up against this with the use of ESO ClusterSecretStore mapping + AWS Load Balancer Controller readiness gates support.

[teebu]

What's the status of this?

[rufreakde]

What did this merge change exactly? https://github.com/isac322/homelab/pull/3/files

[danieljkemp]

Another use for setting namespace labels: Pod Security Admission labels for namespaces

Will likely wait for multi-source applications in 2.5 (or 2.6...)

[czhujer]

FYI I'm currently solving PSS/PSA annotations and looks like kyverno has policies for this..

[rsmets]

What did this merge change exactly? https://github.com/isac322/homelab/pull/3/files

@rufreakde looks like @isac322 was just referencing a workaround he did, which was to create an Argo app to handle defining a namespace with labels.

Anyways, I am anxiously awaiting support for this: labels definitions in the create namespace definition.

[jasonhao518]

also want the same feature, in my case I want the new namespace to be labeled with istio-injection=true, to make istio sidecar injected.

[jyotibhanot]

Is this issue resolved? We are also trying to inject some labels in namespace elbv2.k8s.aws/pod-readiness-gate-inject=enabled but don't know how to do with argo cd. @nickjj : Were you able to resolve the issue with argo cd?

[nickjj]

Were you able to resolve the issue with argo cd?

Not with Argo CD directly, I ended up creating the namespaces before setting up Argo CD using Terraform and then configured the labels there.

[JasonHao123]

My workaround is similar, to create namespace in another argocd application before hand.

[ledroide]

I ended up creating the namespaces before setting up Argo CD

This workaround can't satisfy our use case. Our ArgoProjects are allowed to create namespaces from Applications specs if matching a specific pattern, then the deployment is fully automated. After the branch merges, there is no "human" operation in the "continuous deployment" process. The app.yaml is applied/created in argocd namespace from a gitOps mechanism, then argoCD creates the app's namespace, and deploys all ressources for a running full-stack that depends on Rolebindings -> Roles -> PodSecurityPolicies.

But very soon (in Kubernetes 1.25), PodSecurityPolicies will be deprecated (actually they are since 1.21 but still supported), and we will need namespaces labels, otherwise the pods won't start at all, because not allowed for the default Pod Security Standards in our clusters.

[jyotibhanot]

@ledroide : How are you adding the labels in argocd?

[ledroide]

@jyotibhanot

How are you adding the labels in argocd?

ArgoCD does not set labels on namespaces. I have tried to deploy a Namespace manifest with labels, but it conflicts with the namespace creation/sync that requires specific privileges, and moreover it loops on deleting the namespace and its contents when you need to sync.prune. See messages above for the explanation.

[crenshaw-dev]

I think this is probably a duplicate of https://github.com/argoproj/argo-cd/issues/7799?

[pasha-codefresh]

@crenshaw-dev i also have draft for this issue, i am working on opening PRs on it

[jyotibhanot]

@nickjj : This is a off the track question but do you have any idea what should be the value of deregistration delay, terminationgraceperiod and sleep in order to achieve zerodowntime deployments. I am following this doc : https://easoncao.com/zero-downtime-deployment-when-using-alb-ingress-controller-on-amazon-eks-and-prevent-502-error/

[crenshaw-dev]

@pasha-codefresh, @ashutosh16 also has draft gitops-engine and argo-cd PRs up, so may the best PR win. 😆

[nickjj]

@jyotibhanot I think this thread is way OT to have that discussion since it'll ping everyone, but I use https://github.com/foriequal0/pod-graceful-drain to do zero downtime deploys without worrying about lifecycle sleep times and graceful timeouts. Feel free to email me (it's in my GH profile) instead of replying here.

[ashutosh16]

@pasha-codefresh I'm thinking of adding new key to syncOption NamespaceMetaData with Json as a value that contain namespace metadata like annotation, and labels. WDYT?

syncOptions:
    - PrunePropagationPolicy=background
    - CreateNamespace=true
    - NamespaceMetaData={\"annotations\" :{ \"fake.annotation.io/fake1\": \"v1\"}, " +
        "\"labels\" : { \"label\": \"v1\" } }

[pasha-codefresh]

@ashutosh16 i just opened my draft prs:

https://github.com/argoproj/gitops-engine/pull/443 https://github.com/argoproj/argo-cd/pull/10288

I think about such addition to sync options

  syncPolicy:
    automated:
      prune: true
      selfHeal: false
    syncOptions:
      - Replace=false
      - PruneLast=false
      - Validate=true
      - CreateNamespace=true
      - ApplyOutOfSyncOnly=false
    createNamespaceLabels:
      lk: lv

[ashutosh16]

@pasha-codefresh I like your approach of using callback function in that way - label & annotation are set in argocd. Will you be able to expand the createNamespaceLabels to more generic ones that can support other namespace objects - like annotations, and labels, and also support multiple labels and annotations?

[ledroide]

@ashutosh16 : I totally agree. Namespaces annotations are often used by operators for various behaviors. createNamespaceAnnotations is obviously relevant, and therefore should be supported within createNamespaceLabels

[pre]

For what it’s worth - i suggest the naming would follow declarative naming conventions.

The term ”create” is imperative and leaves undefined what would happen after the first create. Should the attributes defined in this be relevant during the whole life-time of the resource or only in the initial ”create” action.

in other words - i suggest the attributes be named in such a way that they describe the whole set of desired attributes. This allows pruning un-used attributes during the lifecycle. Declarative set also allows the namespace itself become outOfSync if labels no longer match at a given point of time - whereas ”create” would leave this undefined or require another ”syncPlease: true” kind of mitigation.

[pasha-codefresh]

Sure, i will extend it with annotations today, should not be complicated, @ashutosh16

[pasha-codefresh]

What about such structure ?

syncPolicy: 
  automated: 
    prune: true
    selfHeal: false
  createNamespaceMetadata: 
    annotations: 
      annotation1: av
      annotation2: av
    labels: 
      label1: lv
      label2: lv
  syncOptions: 
    - Replace=false
    - PruneLast=false
    - Validate=true
    - CreateNamespace=true
    - ApplyOutOfSyncOnly=false

[crenshaw-dev]

I like this. I'm a little worried that, at a glance, namespaceMetadata might be understood as "metadata applied to all namespaces managed by the App" (not just those auto-created). How about createNamespaceMetadata? It's clunky, but maybe slightly less ambiguous?

[pasha-codefresh]

yes, that's what i thought initially :) changing

[pasha-codefresh]

@crenshaw-dev @ashutosh16 i have updated PR, please take a lot, it is not the final version ( missed unit tests ) , want make sure that logic is correct here first. Thanks

https://github.com/argoproj/argo-cd/pull/10288 https://github.com/argoproj/gitops-engine/pull/443

[crenshaw-dev]

Added one comment on the gitops PR.

[rassie]

@nickjj : If you define the namespace resource in a yaml manifest, and set a spec.syncPolicy.automated.prune=true in your argoproj.io/application resource, then there will be a namespace prune/creation loop every time you commit any change.

@ledroide I've added the argocd.argoproj.io/sync-options: Prune=false annotation on the namespace and don't seem to see any namespace pruning. Would you consider this an acceptable workaround for people needing namespace annotations right now?

[crenshaw-dev]

Closing this in favor of (I think) duplicate https://github.com/argoproj/argo-cd/issues/7799

[rassie]

Closing this in favor of (I think) duplicate #7799

@crenshaw-dev I'm sorry if I'm misunderstanding something, but are you sure you intended to close this issue? The issue number is the same and besides that I was under an impression that this issue was forthcoming, with PRs by @pasha-codefresh reviewed by yourself.

[pasha-codefresh]

Yeah, also not sure why it was closed, probably we have duplication but @crenshaw-dev put wrong link

[crenshaw-dev]

Apologies, I meant https://github.com/argoproj/argo-cd/issues/6215 :-)

[leoluz]

@pasha-codefresh

What about such structure ?

syncPolicy: 
  automated: 
    prune: true
    selfHeal: false
  createNamespaceMetadata: 
    annotations: 
      annotation1: av
      annotation2: av
    labels: 
      label1: lv
      label2: lv
  syncOptions: 
    - Replace=false
    - PruneLast=false
    - Validate=true
    - CreateNamespace=true
    - ApplyOutOfSyncOnly=false

As stated by @pre above I would suggest to change the terminology to avoid confusion. Maybe instead of createNamespaceMetadata we could have something like managedNamespaceMetadata ? Those values should be reconciled with the cluster state if the values in the Application changes and not just when the namespace is first created as the current name implies.

We would end up with something like:

syncPolicy: 
  automated: 
    prune: true
    selfHeal: false
  managedNamespaceMetadata: // or maybe just namespaceMetadata
    annotations: 
      annotation1: av
      annotation2: av
    labels: 
      label1: lv
      label2: lv
  syncOptions: 
    - Replace=false
    - PruneLast=false
    - Validate=true
    - CreateNamespace=true // this is legacy so maybe this can stay as is
    - ApplyOutOfSyncOnly=false

[pre]

As a term for namespace, managedXyz sounds better than createXyz.

Concrete example: add label for istio-injection: enabled which enables the istio sidecar injection. If terminology was createXyz what would happen when this label is removed from Application defined labels?

In other words: it should be possible to also remove a label which is set by argocd Application labels. With createXyz a delete action sounds backwards, but with managedXyz it should be that these are the only labels for that namespace and argocd conciles the cluster state to match these labels or otherwise namespace state becomes outOfSync. Otherwise it’s not possible to remove labels in a deterministic manner.

As of today, our team is jumping through hoops to get the istio-injection: enabled defined in the namespace created by argocd. But we need this label to also stay there, so that if it goes missing, argocd reconciles it back. In this view the imperative createXyz approach is not enough, but declarative managedXyz is.

[joyartoun]

I am also very keen to seeing this come out. We are using argocd in openshift 4.11 and we are deploying a lot of different operators which in one way or another needs to use privileged SCC. Openshift 4.12 will enforce security violation policies as of openshift 4.12 and will terminate pods which are run with privileged. You can apparently disable this function by adding a label to a namespace.

https://cloud.redhat.com/blog/pod-security-admission-in-openshift-4.11

any updates on when this will be out? specifically in operatorhub (which we deploy argo from)

[ledroide]

@joyartoun : I have finally used a workaround to add specific metadata to namespaces at creation time, for namespaces matching a specific regexp. I use OpenPolicyAgent GateKeeper, that embed a mutation webhook. When ArgoCD creates the namespace, it asks the apiserver, so the mutation webhook comes here and changes the api call to add a label to the Namespace resource.

It looks like this :

apiVersion: mutations.gatekeeper.sh/v1beta1
kind: AssignMetadata
metadata:
  name: namespace-labels-myapp
spec:
  match:
    kinds:
      - apiGroups:
          - ""
        kinds:
          - Namespace
    name: myapp*
  location: metadata.labels."pod-security.kubernetes.io/enforce"
  parameters:
    assign:
      value: restricted

Hope it helps

[blakepettersson]

An updated PR (draft) can be found here: #10672 and argoproj/gitops-engine#465

[amfred]

I'm looking forward to this as well. I'm trying to add network policy audit logs, and to do that I need to annotate my ArgoCD-created namespaces per https://docs.openshift.com/container-platform/4.11/networking/network_policy/logging-network-policy.html