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

drop support for argocd-cm Config Management Plugins in favor of sidecars #8117

Closed
crenshaw-dev opened this issue Jan 7, 2022 · 12 comments · Fixed by #13755
Closed

drop support for argocd-cm Config Management Plugins in favor of sidecars #8117

crenshaw-dev opened this issue Jan 7, 2022 · 12 comments · Fixed by #13755
Assignees
Labels
breaking/low A possibly breaking change with low impact enhancement New feature or request security Security related
Milestone

Comments

@crenshaw-dev
Copy link
Member

crenshaw-dev commented Jan 7, 2022

Summary

Argo CD should drop support for CMPs installed via argocd-cm. Installing CMPs via sidecars on repo-server should be made as simple as possible (via docs/examples).

Motivation

  1. Having two ways to configure CMPs is confusing.

    The docs are longer, and it's difficult to tell which comments apply to which install method.

  2. Having two ways to use CMPs is confusing.

    argocd-cm CMPs must be referenced by name in the Application spec. But sidecar CMPs must not be referenced by name (they're invoked only according to discovery output). This difference is difficult to document (each place where there's an example plugin config must include a comment) and is not intuitive to users (why would one plugin be referenced by name and another plugin not be referenced by name)?

  3. Maintaining two CMP installation methods is difficult and error-prone.

    The differing invocation methods is one example. Differing manifest init processes is another example.

  4. Providing support for two different CMP types is difficult.

    For each question asked about CMPs, the answers may differ based on what kind of plugin. The extra back-and-forth and clarification of terms can introduce frustration.

  5. Adding binaries via volumeMount or a custom repo-server image is weird.

    The volumeMount relies on pulling files from an external source. The custom image requires a custom build/update process.

  6. Decoupling the repo-server from plugins just makes sense.

    Generating manifests is a distinct task from the other work the repo-server does. Adding separation between repo-server and plugins can help avoid unexpected interaction, especially since the config management tools interact heavily with the filesystem.

Proposal

  1. Deprecate argocd-cm CMPs.
    1. Remove documentation about how to install them.
    2. Add a warning to logs and the UI when an argocd-cm plugin is enabled. Note the version when the feature will be removed.
  2. Write a migration guide for modifying an argocd-cm CMP to be a sidecar CMP.
  3. Remove the feature in some major release.
@crenshaw-dev crenshaw-dev added enhancement New feature or request breaking/low A possibly breaking change with low impact labels Jan 7, 2022
@pmorch
Copy link

pmorch commented Jan 20, 2022

Do see #8243 before deprecating argocd-cm plugins.

@crenshaw-dev crenshaw-dev added the security Security related label Apr 18, 2022
@crenshaw-dev crenshaw-dev added this to the v2.5 milestone Jun 6, 2022
crenshaw-dev added a commit to crenshaw-dev/argo-cd that referenced this issue Aug 15, 2022
Signed-off-by: CI <michael@crenshaw.dev>
@crenshaw-dev crenshaw-dev modified the milestones: v2.5, v2.6 Aug 15, 2022
@crenshaw-dev
Copy link
Member Author

Deprecation will happen in 2.5, actually removing support will be in 2.6.

leoluz pushed a commit that referenced this issue Aug 17, 2022
* chore: deprecate argocd-cm plugins (#8117)

Signed-off-by: CI <michael@crenshaw.dev>

* more warnings

Signed-off-by: CI <michael@crenshaw.dev>

* more warnings

Signed-off-by: CI <michael@crenshaw.dev>

Signed-off-by: CI <michael@crenshaw.dev>
@34fathombelow 34fathombelow moved this to To be assigned in Argo CD Roadmap Nov 9, 2022
@34fathombelow 34fathombelow moved this to Backlog in Argo CD Roadmap Nov 10, 2022
@34fathombelow 34fathombelow moved this to Backlog in Argo CD Roadmap Nov 10, 2022
@crenshaw-dev crenshaw-dev self-assigned this Nov 17, 2022
@crenshaw-dev crenshaw-dev removed this from the v2.6 milestone Nov 17, 2022
@crenshaw-dev
Copy link
Member Author

If this gets merged in time for 2.6, I think we should delay argocd-cm removal until 2.7 so there's a transition release for folks needing the plugin.name feature.

@crenshaw-dev crenshaw-dev added this to the v2.7 milestone Dec 12, 2022
@koushyk
Copy link

koushyk commented Jan 24, 2023

But how multitenancy will be handled now?

@crenshaw-dev
Copy link
Member Author

@koushyk I understand that Argo CD Vault Plugin documentation describes a multi-tenancy design involving CMPs, but I don’t know the details. As far as I know, sidecar plugins can accomplish the same thing. What problems do you anticipate?

@koushyk
Copy link

koushyk commented Jan 24, 2023

@koushyk I understand that Argo CD Vault Plugin documentation describes a multi-tenancy design involving CMPs, but I don’t know the details. As far as I know, sidecar plugins can accomplish the same thing. What problems do you anticipate?

I know there is a way to pass variable to using plugin:name:env but what option I have with sidecar plug-ins ?

@crenshaw-dev
Copy link
Member Author

I'm not sure I follow... but sidecar plugins support the same name and env fields that argocd-cm plugins support.

@koushyk
Copy link

koushyk commented Jan 24, 2023

I'm not sure I follow... but sidecar plugins support the same name and env fields that argocd-cm plugins support.

it's working without specifying name:

spec:
  source:
    plugin:
      env:
        - name: FOO
          value: bar

@crenshaw-dev
Copy link
Member Author

As of Argo CD 2.6, sidecar plugins support plugin specifications either with or without name.

If name is specified, we use the specified plugin. If there are any "discovery" rules on the plugin, we'll check those as well.

If name is not specified, we loop over the configured plugins and use the first one whose discovery rules pass.

So, as far as I can tell, sidecar plugins have at least as many capabilities as argocd-cm plugins. The discovery rules (and therefore name-less plugin specifications) are just additional features which are available if you choose to use them.

@crenshaw-dev
Copy link
Member Author

Considering pushing the feature removal to 2.8. There are still some issues with sidecar CMPs I'd like to sort.

@jemag
Copy link
Contributor

jemag commented Feb 17, 2023

If deprecation is pushed to 2.8, is there any chance to also target for 2.8 some kind of generic mechanism for on-demand sidecar access to specific private helm repos/git credentials. (e.g: having a generic mechanism to handle cases such as #8820 and #10265)

@mprusinski
Copy link

If deprecation is pushed to 2.8, is there any chance to also target for 2.8 some kind of generic mechanism for on-demand sidecar access to specific private helm repos/git credentials. (e.g: having a generic mechanism to handle cases such as #8820 and #10265)

Maybe sidecar doesn't exactly need to have access to private repos? I'm struggling with integrating Argo Vault Plugin with helm and faced the same issue (private repo not visible by the sidecar plugin) and what have crossed my mind is an idea to allow some sort of pipeline mechanism when using plugins (like a list of plugins to generate final output applied to cluster). In such case helm built-in plugin would be run by ARGO (with access to prive repo) and the AVP would only apply its logic on the output generated by helm plugin.

@agaudreault agaudreault moved this from Backlog to Completed in Argo CD Roadmap Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking/low A possibly breaking change with low impact enhancement New feature or request security Security related
Projects
Status: Completed
Development

Successfully merging a pull request may close this issue.

5 participants