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

Propagate Application labels to application manifest as way to define… #11686

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

brat002
Copy link

@brat002 brat002 commented Dec 13, 2022

Propagate Application labels to application manifest as way to define environment-specific values for monitoring, logging, incident-management

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).

… environment-specific values for monitoring, logging, incident-management
@brat002
Copy link
Author

brat002 commented Dec 13, 2022

Please help to adapt this MR for requirements. This is very useful feature.

@ashutosh16
Copy link
Contributor

@brat002 Can you provide the docs for this feature?

@brat002
Copy link
Author

brat002 commented Dec 14, 2022

Done. @ashutosh16 is it enough? Should I add the documentation somewhere else?

@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Base: 46.96% // Head: 46.96% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (3fc8a52) compared to base (c4c6bfa).
Patch coverage: 40.00% of modified lines in pull request are covered.

❗ Current head 3fc8a52 differs from pull request most recent head f85c779. Consider uploading reports for the commit f85c779 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11686      +/-   ##
==========================================
- Coverage   46.96%   46.96%   -0.01%     
==========================================
  Files         243      243              
  Lines       40424    40430       +6     
==========================================
+ Hits        18985    18986       +1     
- Misses      19529    19534       +5     
  Partials     1910     1910              
Impacted Files Coverage Δ
reposerver/repository/repository.go 60.28% <16.66%> (-0.17%) ⬇️
util/argo/resource_tracking.go 69.56% <66.66%> (ø)
util/kube/kube.go 69.47% <100.00%> (ø)
util/settings/settings.go 48.42% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@blakepettersson
Copy link
Member

Do we really want to automatically propagate all Application labels to application manifests? Why can't source Kustomizations or Helm values be used for those purposes? Could not ApplicationSet templating not be used for source/cluster specific customisations?

@brat002
Copy link
Author

brat002 commented Dec 27, 2022 via email

@blakepettersson
Copy link
Member

Something which I think could be interesting would be to selectively apply labels by Kind and Group, something like

managedResourceMetadata:
  labels:
    foo-label:
      value: bar
      any:
      - resources:
          names: 
          - "prod-*"
          - "staging"
          kinds:
          - Service       

(got some inspiration from how Kyverno does this).

This would need some fleshing out if this is something that would even be desirable - perhaps @jannfis or @crenshaw-dev have some thoughts on this?

@crenshaw-dev
Copy link
Member

I feel like maybe we could accomplish the same thing by adding a source.patches field mimicking Kustomize's behavior.

@blakepettersson
Copy link
Member

@crenshaw-dev it seems like #14648 could work for this. Could this be combined with Helm charts being rendered with helm template? Or is this something that would need #12508 (if combining helm and kustomize renderings is even sane)?

@crenshaw-dev
Copy link
Member

@blakepettersson yeah, #14648 would work if you --enable-helm. But you'd miss out on Argo CD's built-in Helm features.

@blakepettersson
Copy link
Member

For Helm sources, #16749 seems like it could be a viable alternative

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.

5 participants