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

feat: Support AVP installed as CMP sidecar #352

Merged
merged 2 commits into from
Jun 28, 2022
Merged

feat: Support AVP installed as CMP sidecar #352

merged 2 commits into from
Jun 28, 2022

Conversation

jkayani
Copy link
Member

@jkayani jkayani commented Jun 13, 2022

Description

Argo CD has supported sidecar based CMP (config management plugin) installation for a while now as part of the V2 proposal argoproj/argo-cd#5927. This PR adds a Kustomize app and docs for using AVP this way, and mentions some caveats associated with doing so. I think it's worth having this information especially b/c it seems argocd-cm based CMP installation might be deprecated in the future: argoproj/argo-cd#8117.

I tested this out on the Kubecon demo repo: https://github.com/jkayani/avp-demo-kubecon-2021/tree/sidecar - this should all be reproducible locally using it.

Checklist

Please make sure that your PR fulfills the following requirements:

  • Reviewed the guidelines for contributing to this repository
  • The commit message follows the Conventional Commits Guidelines.
  • Tests for the changes have been updated
  • Are you adding dependencies? If so, please run go mod tidy -compat=1.17 to ensure only the minimum is pulled in.
  • Docs have been added / updated
  • Optional. My organization is added to USERS.md.

Type of Change

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • New tests
  • Build/CI related changes
  • Documentation content changes
  • Other - Kustomize YAML changes

Other information

@jkayani jkayani self-assigned this Jun 13, 2022
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Outdated Show resolved Hide resolved
docs/usage.md Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #352 (30ea752) into main (45a4c25) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #352   +/-   ##
=======================================
  Coverage   77.53%   77.53%           
=======================================
  Files          22       22           
  Lines        1006     1006           
=======================================
  Hits          780      780           
  Misses        142      142           
  Partials       84       84           

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@jkayani jkayani mentioned this pull request Jun 13, 2022
@jkayani jkayani marked this pull request as ready for review June 14, 2022 04:32
@jkayani jkayani requested a review from werne2j as a code owner June 14, 2022 04:32
Copy link
Member Author

@jkayani jkayani left a comment

Choose a reason for hiding this comment

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

Reading over https://blog.argoproj.io/breaking-changes-in-argo-cd-2-4-29e3c2ac30c9 it looks like starting with 2.4.0, environment variables set at the application manifest get the ARGOCD_ENV prefix for argod-cm ConfigMap installed plugins, not just sidecars. So I guess existing docs need to be updated, and maybe the code should accept vars with or w/o that prefix (so AVP_TYPE or ARGOCD_ENV_AVP_TYPE)

Edit: This is all true but can probably be done in the scope of a second PR, this one can just focus on the sidecar support starting at 2.4.0

@werne2j
Copy link
Member

werne2j commented Jun 16, 2022

Reading over https://blog.argoproj.io/breaking-changes-in-argo-cd-2-4-29e3c2ac30c9 it looks like starting with 2.4.0, environment variables set at the application manifest get the ARGOCD_ENV prefix for argod-cm ConfigMap installed plugins, not just sidecars. So I guess existing docs need to be updated, and maybe the code should accept vars with or w/o that prefix (so AVP_TYPE or ARGOCD_ENV_AVP_TYPE)

Edit: This is all true but can probably be done in the scope of a second PR, this one can just focus on the sidecar support starting at 2.4.0

Yeah, i guess we should put something in about not working with 2.4 yet. Or at least just create an issue.

docs/compatibility.md Outdated Show resolved Hide resolved
@jkayani jkayani merged commit 08bfa36 into main Jun 28, 2022
@jkayani jkayani deleted the cmp-sidecar branch June 28, 2022 16:07
@timofey-drozhzhin
Copy link

@jkayani for cmp-sidecar/cmp-plugin.yaml it might be a good idea to append -maxdepth 1 to the discover commands to prevent subdirectories from falsely triggering the plugin.

Also, for the helm plugin, it might not be necessary to require the values.yaml file, since some users may want to just use Chart.yaml for dependencies.

jskrzypek added a commit to jskrzypek/argocd-vault-plugin that referenced this pull request Jul 7, 2022
PR argoproj-labs#352 introduced the sidecar pattern, but also renamed the prior default
manifest, which was available through a reference to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests

To get the same app, consumers would need to change any exsting references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap

This commit introduces a simple proxy manifest that makes that change backwards
compatible, meaning that now older references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests
will still resolve to the same app state as
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap
rather than just breaking.
jskrzypek added a commit to jskrzypek/argocd-vault-plugin that referenced this pull request Jul 7, 2022
PR argoproj-labs#352 introduced the sidecar pattern, but also renamed the prior default
manifest, which was available through a reference to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests

To get the same app, consumers would need to change any exsting references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap

This commit introduces a simple proxy manifest that makes that change backwards
compatible, meaning that now older references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests
will still resolve to the same app state as
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap
rather than just breaking.
jskrzypek added a commit to jskrzypek/argocd-vault-plugin that referenced this pull request Jul 7, 2022
PR argoproj-labs#352 introduced the sidecar pattern, but also renamed the prior default
manifest, which was available through a reference to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests

To get the same app, consumers would need to change any exsting references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap

This commit introduces a simple proxy manifest that makes that change backwards
compatible, meaning that now older references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests
will still resolve to the same app state as
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap
rather than just breaking.
jskrzypek added a commit to jskrzypek/argocd-vault-plugin that referenced this pull request Jul 7, 2022
PR argoproj-labs#352 introduced the sidecar pattern, but also renamed the prior default
manifest, which was available through a reference to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests

To get the same app, consumers would need to change any exsting references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap

This commit introduces a simple proxy manifest that makes that change backwards
compatible, meaning that now older references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests
will still resolve to the same app state as
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap
rather than just breaking.
jskrzypek added a commit to jskrzypek/argocd-vault-plugin that referenced this pull request Jul 11, 2022
PR argoproj-labs#352 introduced the sidecar pattern, but also renamed the prior default
manifest, which was available through a reference to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests

To get the same app, consumers would need to change any exsting references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap

This commit introduces a simple proxy manifest that makes that change backwards
compatible, meaning that now older references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests
will still resolve to the same app state as
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap
rather than just breaking.
jskrzypek added a commit to jskrzypek/argocd-vault-plugin that referenced this pull request Jul 12, 2022
PR argoproj-labs#352 introduced the sidecar pattern, but also renamed the prior default
manifest, which was available through a reference to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests

To get the same app, consumers would need to change any exsting references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap

This commit introduces a simple proxy manifest that makes that change backwards
compatible, meaning that now older references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests
will still resolve to the same app state as
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap
rather than just breaking.
jskrzypek added a commit to jskrzypek/argocd-vault-plugin that referenced this pull request Jul 15, 2022
PR argoproj-labs#352 introduced the sidecar pattern, but also renamed the prior default
manifest, which was available through a reference to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests

To get the same app, consumers would need to change any exsting references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap

This commit introduces a simple proxy manifest that makes that change backwards
compatible, meaning that now older references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests
will still resolve to the same app state as
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap
rather than just breaking.
jskrzypek added a commit to jskrzypek/argocd-vault-plugin that referenced this pull request Aug 24, 2022
PR argoproj-labs#352 introduced the sidecar pattern, but also renamed the prior default
manifest, which was available through a reference to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests

To get the same app, consumers would need to change any exsting references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap

This commit introduces a simple proxy manifest that makes that change backwards
compatible, meaning that now older references to
https://github.com/argoproj-labs/argocd-vault-plugin//manifests
will still resolve to the same app state as
https://github.com/argoproj-labs/argocd-vault-plugin//manifests/cmp-configmap
rather than just breaking.
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.

None yet

4 participants