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

Add default ArgoCD label to auto created namespaces #6215

Closed
ghost opened this issue May 11, 2021 · 16 comments · Fixed by #10672
Closed

Add default ArgoCD label to auto created namespaces #6215

ghost opened this issue May 11, 2021 · 16 comments · Fixed by #10672
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ghost
Copy link

ghost commented May 11, 2021

Summary

Currently there is no way to see which namespaces were automatically created by ArgoCD. Goal is to add a default label like argocd.argoproj.io/namespace: production.

Motivation

There are a couple of reasons I can honesyly think of:

  1. This allows you to easily see which namespaces were created by ArgoCD or manually by a user with a bit to much power (CR/R)
  2. You can reference this label in your network policies (namedpace selectors) OOTB, this makes the auto create feature even more powerful
  3. In the event you want to empty/clean all namespaces and associated resources with, you can easily use kubectl combined with a label selector

Proposal

Add a default label like argocd.argoproj.io/namespace: production where production here would be the actual name of namespace off course.

@ghost ghost added the enhancement New feature or request label May 11, 2021
@alexmt alexmt added this to the v2.1 milestone Jun 9, 2021
@alexmt alexmt modified the milestones: v2.1, v2.2 Jul 2, 2021
@jaideepr97
Copy link
Contributor

jaideepr97 commented Aug 4, 2021

Hi! I'd be interested in taking a shot at this
cc @alexmt

@jaideepr97
Copy link
Contributor

Hey @Tpx01 do you mind sharing some use cases where argo-cd creates its own namespaces? Might help with testing etc

@ghost
Copy link
Author

ghost commented Aug 10, 2021

Hey @Tpx01 do you mind sharing some use cases where argo-cd creates its own namespaces? Might help with testing etc

Sure thing! One of the easiest things to do is use the helm example: https://github.com/argoproj/argocd-example-apps/tree/master/helm-guestbook

Next, when creating the application you can enable the checkbox in ArgoCD (upon creation of the application) to auto-create namespaces as depicted in the printscreen.

Screenshot_20210810-202210

By enabling this checkbox ✅ the routine/go-code, responsible for the creation of namespaces, will be called. The namespace that will be created is the same as referenced in the destination namespace of your newly Application CRD.

Does this help?

@jaideepr97
Copy link
Contributor

Sure does, thanks!

@jessesuen
Copy link
Member

jessesuen commented Aug 12, 2021

This was discussed in today's contributors meeting. Some points:

  1. Aside from the resource tracking use case mentioned in the description, namespace labeling/annotations are used in other use cases such as: sidecar injection (OPA, istio, vault, aws load balancer). If we do decide to support this feature, it would need be flexible enough to support arbitrary labels and annotations (e.g. istio-injection=enabled)

  2. Since labeling/annotations would be different from namespace to namespace, this would need to be done at a granular level (e.g. possibly Application or Project level). We felt a system-level set of labels/annotations would not be flexible enough, and that it might be too much to introduce a concept of "namespace templates" .

  3. Auto creation of namespace feature was originally created to support the helm install --create-namespace use case. However, helm install --create-namespace does not provide any convenience about additional labels/annotations. There is an argument that namespace creation should be intentionally left to be limited. And if labeling/annotating namespaces is needed, then it should be performed through normal configuration management.

@rayterrill
Copy link

Having the ability to add labels/annotations to namespaces created with createNamespace is something we're definitely looking for 🙏 - currently running into some issues with this because we need to add labels to namespaces created implicitly with createNamespace.

@aaronthebaron
Copy link

In our App of Apps, Multi-Cluster model, where we use namespaces to segregate our application stacks into dev|stag|prod environments, we currently are just adding another App that only creates the namespace with istio-injection labels, which is our use case.

This solution clutters the Argo UI and isn't very DRY, but works. The multicluster part makes the App required, because it's the only way we can control namespaces being deployed into remote clusters along with our selection of applications that define our application stack.

This feature would be a great way to simplify what is otherwise a very automated, easy to use setup for creating ephemeral application environments (Thanks ArgoCD). If it helps to bring this feature to life, I would be more than happy to describe this use case in more detail, but we too would find it very useful.

@alexmt alexmt modified the milestones: v2.2, v2.3 Dec 8, 2021
@KelvinVenancio
Copy link

+1 Completely agree! Special if we are working with Istio, for example.

@ledroide
Copy link

ledroide commented Jan 6, 2022

I'm also looking ad labelled namespaces for NetworkPolicies.
When ArgoCD creates a namespace, there is no label that I can use in a NetworkPolicy, except kubernetes.io/metadata.name, but it can be used only once (because the same as namespace name), and cannot be "product=myapp"

@sidh
Copy link

sidh commented Jan 18, 2022

We too are looking for labelled namespaces. It is required to enable automatic pod readiness gates for AWS LoadBalancer Controller - https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.1/deploy/pod_readiness_gate/.

@ledroide
Copy link

ledroide commented Feb 2, 2022

Another use case for namespace labels : Pod Security Standards.

apiVersion: v1
kind: Namespace
metadata:
  name: ns-created-by-argocd
  labels:
    pod-security.kubernetes.io/enforce: baseline
    pod-security.kubernetes.io/audit: restricted

This feature replaces PodSecurityPolicies (deprecated) and requires namespace labels to be set
Namespace labels becomes a mandatory feature for future apps in Kubernetes 1.23+.

@BadLiveware
Copy link

This would also help assigning namespaces to projects in rancher which uses annotations

# Create Namespace
apiVersion: v1
kind: Namespace
metadata:
  name: <namespace>
  annotations: 
    field.cattle.io/projectId: <project-id>

So a way to connect argocd projects to rancher projects without managing them separately would be very helpful

@v1r7u
Copy link

v1r7u commented Feb 18, 2022

In our environment, we use multiple argocd applications inside the same namespace and we also need custom labels to be attached on namespaces. However, I think that adding labels together with namespace auto-creation could cause significant headaches in edge cases:

  • if we have several argocd Applications that are deployed to the same namespace, should all of them have createNamespace=true or only one?
  • if several Applications have a different set of labels, should argocd merge them or report an issue?
  • if you change the set of labels on an already created namespace, should argocd apply the change?
  • if you delete a label, should argocd delete it too?

And different combinations of all the above.

At the moment, I tend to think that if we need specific labels - it is better to track them as a separate namespace object with a separate lifecycle from the Application itself.

@ledroide
Copy link

Hello @v1r7u . Good questions. But some use cases look weird, specific, or complicated to me.

argocd Applications that are deployed to the same namespace, should all of them have createNamespace=true or only one?

Simple answer : if one of the has the createNamespace=true flag, even if other Apps don't, then the namespace should be created (if the AppProj allows this resource).

different set of labels, should argocd merge them or report an issue?

If you have 1 App per namespace, this should not happen. If 2 Apps deploy to the same namespace with different namespace labels, then merge.

If namespace labels conflict (same label with different values), then there will be a synchronization loop, and the user will need to find out the conflicting label, and fix it.

if you change/delete the set of labels on an already created namespace

Then change/delete the namespace labels when next sync, as expected.

@v1r7u
Copy link

v1r7u commented Feb 21, 2022

Hello, @ledroide!
Thanks for your answers!

Most of our problems will arise when we will need to understand how the namespace was created. We have multiple relatively big repositories that are used and maintained by several independent teams. And all the time we have several Applications deployed to the same namespace.

With the proposed model we increase the cognitive load on a human. I might have several Applications deployed to the same namespace, but each of these Applications might have different namespace-related parameters: some say createNamespace=false, some say true but without labels, and the last one adds labels on the top. Then the human operator has to aggregate all of these pieces into a single final object and debug why the actual object slightly differs from their expectations.

I believe, that impact of this proposal could be controlled by raising correct engineering culture: contributing guides, PR reviews and validation, knowledge-sharing, etc. But I think the best way to avoid issues is to limit ways how you might end up in a trouble. As an example, you can constantly educate your users to set resources requests/limits, but in my opinion, it's better to accompany the guide with a validation webhook that forbids creating pods without resources. The same with this proposal: we might write a guide on how to use createNamespace= true with default labels, but often it's a matter of time when someone accidentally does a change that does not follow the guide.

@ledroide
Copy link

@v1r7u : There will always exist tricky use cases like yours. It should not prevent however from offering simple solutions that satisfy most use cases.

About managing resources.requests/limits, the solution is to set Limit Ranges for your pods in the namespace, combined with Resource Quotas

@crenshaw-dev crenshaw-dev modified the milestones: v2.5, v2.6 Oct 4, 2022
blakepettersson added a commit to blakepettersson/argo-cd that referenced this issue Nov 4, 2022
This builds upon the work that @pasha-codefresh did in argoproj#10288.

The main differences between this PR and the previous one is that we use
SSA to diff between different versions of the namespace, as well as
having a slightly different API in gitops-engine for setting the
namespace modifier.

We now also set the ownership of the namespace in ArgoCD.

Closes argoproj#4628
Closes argoproj#6215
Closes argoproj#7799

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
@leoluz leoluz closed this as completed in 7773021 Nov 4, 2022
ashutosh16 pushed a commit to ashutosh16/argo-cd that referenced this issue Nov 23, 2022
* namespace labels

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* create namespace should support annotations

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* handle also modification hook

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* regenerate entity on modify hook

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* manifests

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* feat: enable metadata to be set on namespaces

This builds upon the work that @pasha-codefresh did in argoproj#10288.

The main differences between this PR and the previous one is that we use
SSA to diff between different versions of the namespace, as well as
having a slightly different API in gitops-engine for setting the
namespace modifier.

We now also set the ownership of the namespace in ArgoCD.

Closes argoproj#4628
Closes argoproj#6215
Closes argoproj#7799

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: don't always track namespaces

For now, only allow namespaces managed with `managedNamespaceMetadata`
to have tracking set by Argo. Ideally we'd like new namespaces to also
be tracked by Argo, but there's currently an issue with a failing
integration test.

Also wrap error message if setting the app instance errors on the
namespace.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: always return true with `hasManagedMetadata`

If `hasManagedMetadata` is set, `true` should always be returned.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* docs: add clarifying docs on resource tracking

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* style: pr tweaks

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: re-add label unsetting

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* Update gitops-engine to current master

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

Signed-off-by: pashavictorovich <pavel@codefresh.io>
Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Co-authored-by: pashavictorovich <pavel@codefresh.io>
Co-authored-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
emirot pushed a commit to emirot/argo-cd that referenced this issue Jan 27, 2023
* namespace labels

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* create namespace should support annotations

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* handle also modification hook

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* regenerate entity on modify hook

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* manifests

Signed-off-by: pashavictorovich <pavel@codefresh.io>

* feat: enable metadata to be set on namespaces

This builds upon the work that @pasha-codefresh did in argoproj#10288.

The main differences between this PR and the previous one is that we use
SSA to diff between different versions of the namespace, as well as
having a slightly different API in gitops-engine for setting the
namespace modifier.

We now also set the ownership of the namespace in ArgoCD.

Closes argoproj#4628
Closes argoproj#6215
Closes argoproj#7799

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: don't always track namespaces

For now, only allow namespaces managed with `managedNamespaceMetadata`
to have tracking set by Argo. Ideally we'd like new namespaces to also
be tracked by Argo, but there's currently an issue with a failing
integration test.

Also wrap error message if setting the app instance errors on the
namespace.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: always return true with `hasManagedMetadata`

If `hasManagedMetadata` is set, `true` should always be returned.

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* docs: add clarifying docs on resource tracking

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* style: pr tweaks

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* fix: re-add label unsetting

Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>

* Update gitops-engine to current master

Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>

Signed-off-by: pashavictorovich <pavel@codefresh.io>
Signed-off-by: Blake Pettersson <blake.pettersson@gmail.com>
Signed-off-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Co-authored-by: pashavictorovich <pavel@codefresh.io>
Co-authored-by: Leonardo Luz Almeida <leonardo_almeida@intuit.com>
Signed-off-by: emirot <emirot.nolan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment