Skip to content

[RFC] External Artifact API #5292

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

stefanprodan
Copy link
Member

@stefanprodan stefanprodan commented Apr 8, 2025

This RFC proposes the introduction of a new API called ExternalArtifact that would allow 3rd party controllers to act as a source of truth for the cluster desired state. In effect, the ExternalArtifact API acts as an extension of the existing source.toolkit.fluxcd.io APIs that enables Flux kustomize-controller and helm-controller to consume artifacts from external source types that are not natively supported by source-controller.

@stefanprodan stefanprodan added the area/rfc Feature request proposals in the RFC format label Apr 8, 2025
@stefanprodan stefanprodan force-pushed the rfc-external-artifact branch 4 times, most recently from 1e5bdf1 to 16ba533 Compare April 8, 2025 11:48
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
@stefanprodan stefanprodan force-pushed the rfc-external-artifact branch from 16ba533 to 9751327 Compare April 8, 2025 11:49
Copy link

@jakobmoellerdev jakobmoellerdev left a comment

Choose a reason for hiding this comment

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

Hey there 👋 Thanks for sharing this in Draft State, cool to see such a development!
(FYI @Skarlso @fabianburth who also worked on the proposal)

This looks very similar to https://github.com/openfluxcd / #5058 original proposal with the main difference being that you use the ExternalArtifact instead of any arbitrary SourceRef.

I am wondering how this would be different to having a CRD that pulls OCI artifacts and then referencing those...

Maybe its even worth thinking about standardizing ExternalArtifacts completely to OCI? I think we talked about this in the Flux community call when the original proposal of openflux was discussed.

The main drawback we mentioned back then still persists:
If we have an ExternalArtifact, Flux will always need to reference ExternalArtifact, and users will not be able to use their origin CRDs. In your Example release-controller would work on a GitHubRelease but the Kustomization still contains ExternalArtifact.

At this point, why not think about having release-controller directly download OCIArtifacts, expose a registry endpoint, and then use the already available Flux types?

This is ultimately what caused us to not follow up on the design.

If there are cases when working with an OCIRegistry is not desired, or unfeasible (e.g. working with SFTP and converting that to OCI Artifacts may be unfeasible / undesirable), then I still see value in this CRD even with the indirection in place!

Looking forward to see where this goes! 🎉

@stefanprodan
Copy link
Member Author

stefanprodan commented Apr 8, 2025

If we have an ExternalArtifact, Flux will always need to reference ExternalArtifact, and users will not be able to use their origin CRDs.

@jakobmoellerdev an arbitrary sourceRef would imply opening up RBAC and the Flux CRD to unknown kinds, which would imply a v2 of Flux Kustomization and a v3 of HelmRelease. With ExternalArtifact the Flux RBAC does not changes, as Flux controllers already have all the permissions needed for the source.toolkit.fluxcd.io APIs and instead of dropping the allow list in the Flux CRDs we will just add another owned Kind to that which does not changes the security stance of Flux.

At this point, why not think about having release-controller directly download OCIArtifacts, expose a registry endpoint, and then use the already available Flux types?

Hosting an OCI registry inside the cluster and pushing content to it from a controller is way more challenging and involving than embedding a Go file server in a custom controller. Given that source-controller contains a storage implementation that can easily be copied in any other controller, the effort to expose artifacts to Flux is significant lower. If running a registry inside the cluster is feasible for you, then using OCIRepository is the right way to go, this RFC is for offering an alternative solution which IMO is easier to implement for 3rd party controllers.

@stealthybox
Copy link
Member

stealthybox commented May 23, 2025

an arbitrary sourceRef would imply opening up RBAC and the Flux CRD to unknown kinds, which would imply a v2 of Flux Kustomization and a v3 of HelmRelease. With ExternalArtifact the Flux RBAC does not changes

Controller authors who want extend Flux can add to a flux controller's RBAC themselves with ClusterRole aggregation.
There may be other reasons why we want a shim API within our own namespace (maybe so Notification controller only has to watch a single Kind of list), but RBAC may not be a sufficient reason on its own.

There could be separate security concerns with the code of multiple controller authors all having write access to the same status objects of a shared shim API.
ex: if ControllerA for SourceTypeA writes to the status of ExternalArtifacts for SourceTypeB, a flux controller could be confused into reconciling an incorrect or malicious set of resources.
There may be ways to prevent that sort of issue with admission on UserInfo using new CEL features in Kubernetes, but we don't currently do anything like that, and it would have to be configured per-controller and likely distributed by the add-on author themselves.

@matheuscscp
Copy link
Member

There may be other reasons why we want a shim API within our own namespace (maybe so Notification controller only has to watch a single Kind of list), but RBAC may not be a sufficient reason on its own.

Not sure what you mean by "NC watching a single kind of list", but NC does not watch any resources 🤔 The other Flux controllers send the event payload to NC via HTTP and external webhooks cause the Receiver controller to annotate the Flux resources for requesting out-of-schedule reconciliations. But all of that can be kustomized to work with Flux-external types by just patching the CRDs and RBAC like we do for flux-operator, no code changes are required:

https://github.com/controlplaneio-fluxcd/flux-operator/blob/07a37e692ed2d7ee6b16cd3c8f8231a134a9fbdc/internal/builder/profiles.go#L65-L138

External source controller authors must document how to do this 👆 for their controllers/CRDs.

There could be separate security concerns with the code of multiple controller authors all having write access to the same status objects of a shared shim API. ex: if ControllerA for SourceTypeA writes to the status of ExternalArtifacts for SourceTypeB, a flux controller could be confused into reconciling an incorrect or malicious set of resources.

Not sure I follow this example. I think the proposal here is that ControllerA for SourceTypeA will create an ExternalArtifact object with ownerRef/controllerRef set to an object of SourceTypeA, and only ControllerA will manage this ExternalArtifact object, no other controllers will. Each external controller will manage its own source types and ExternalArtifact object instances for its source types. If you install a malicious controller in the cluster that messes with it, I think that's on you, and that's the risk of opening up Flux for external source controllers regardless of particular details of the implementation we choose?

@matheuscscp
Copy link
Member

I think maybe a strong reason to choose an ExternalArtifact CRD is that we can easily watch it in kustomize-controller. I heard dynamic watches are possible for unknown types, but we are also indexing the managed Kustomizations by source type during manager setup, etc. Not sure how complex/doable/maintainable all of that would be for dynamically watching unknown types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rfc Feature request proposals in the RFC format
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants