Expose GIT_SHA as substitution variable

[kingdonb]

I have built an example which does manifest generation in the way that I think it is intended to be done (in CI) and I found there was a pretty common use case that could be facilitated by a little help that I think kustomize-controller is in position to lend without much (anything?) materially changing in the controller or in the CRD.

Proposal: make GIT_SHA (or FLUX_GIT_SHA) available as a variable that can be substituted at kustomize build time through envsubst. This can be used through the spec.postBuild feature to avoid unnecessary kludges like, a CI job that takes the latest commit hash from a branch, and commits it back to the branch in a configmap like this one:

kingdonb/any_old_app@388dfa3#diff-9105252cca777939348fc91a5a46ce69fa9da0a3346d31747cc9bbbe7ef0f680

apiVersion: v1
data:
  GIT_SHA: 889048a1
kind: ConfigMap
metadata:
  name: any-old-app-version
  namespace: devl

... for use in later postBuild substitutions with substituteFrom.

There is a case that manifest generation tools can probably pick up the slack here, and if they are run in a scenario where they have access to the GIT_SHA, they can write it out in any deployments' image spec while remaining DRY. I have several examples for these kind of scenarios and I plan to add them to the documentation.

But this example in particular seems quite overcomplicated because it does not have access to the GIT_SHA variable at runtime for kustomize build. See #1002 - this is a use case that was very easy in .flux.yaml (it is retrieved in-line since a git checkout is already present) and I've been able to replicate this with help of CI, but it sure looks a lot more complicated than it seems to need, than it was in Flux v1. I don't think you should need a CI task that writes back to git in order to solve this use case. (The git task that writes back to CI is still important, it just seems unnecessary for this particular use case.)

Kustomization might already know its GIT_SHA, or if not, it should be in a position to retrieve it from the source-controller – sometimes you will be applying from a Bucket or other source and it might not be available. And that's fine... there could be more useful variables that would be made available depending on the source and what information it includes.

There is also the possibility that Kustomization spec could include additional variables, like the kustomization's source name along with source ref, or even including variables with information about additional (external) source refs at the time of build, but there are some problems with this model that I won't approach in this proposal. However any Kustomization, assuming a source of a GitRepository, can probably make some fairly strong guarantees of a declarative nature (eg. GIT_SHA will always be the same for a given kustomize build) those guarantees that wouldn't be as strong for external sources that can move independently. (I'm talking about pulling variables from sourceRef eg. an unrelated source. It would be useful for fleet-infra to know the current SHA of a gitrepository source that produces an image through CI, even if that source isn't associated with any downstream kustomization.)

It would be really helpful for the purpose of manifest generation if Kustomization and its build had access to information about itself and the source it's reconciling in the form of making some variables available for use in postBuild substitution.

[Fabian-K]

Just a idea, but what if the GitRepository.status is extended with a field containing the GIT_SHA and variable substitution is extended to target not only Secret and ConfigMaps but any field of a CR?
Alternatively if the GitRepository controller, in addition, creates and updates a ConfigMap similar to its status, there would be no need to extend variable substitution.

[kingdonb]

That's a really interesting idea, but I'm not sure how it would work/if it is practically possible, and actually I don't think that I like it as much. I think secrets and configmaps are targets of substituteFrom because they are guaranteed to be APIs that the cluster knows about, and will have a stable API. Can you really do a safe runtime reflection on a CR that you don't know if it's even present in the cluster? This sounds significantly more involved than just adding a single variable we definitely have (almost definitely though, since the source could be a bucket.)

I used a configmap so that there would be something for downstream Kustomizations to read from, but technically if you have GIT_SHA and you just want to drive deployments inside of the same GitRepository then you can do away with the configmap altogether and just write the variable directly into some deployments. I imagined that the specifics of the deployment are left up to the cluster repo and the app isn't responsible for deployment, only for advertising the latest version of the image in a way that can be used with substituteFrom to avoid repeating the GIT_SHA string in a bunch of different places. (It's not possible to substituteFrom a value in a configmap that hasn't been applied yet. That's the reason I layer one Kustomization on top of the other, because I might have multiple refs to this SHA and I don't want to have to use sed or yaml-updater to rewrite a string all over the place.)

In my example for "deploy the latest image using manifest generation" (the same one that is linked above and referred to again below), the app repo gets a commit, CI triggers two jobs (1) to build an image tag with that GIT_SHA, and (2) to write a commit with the GIT_SHA in it back to the branch after the commit the image is built from, with a configmap including the commit hash that was used for the image tag.

With GIT_SHA available to postBuild, that is drastically simplified to just one job (the image build), dropping the "commit-back" which is no longer required for this use case. Now you just write a configmap that looks like this instead, the CI job to update it goes away, and CI does not need to commit+push anything for this, as Kustomize controller/envsubst together does all the work:

apiVersion: v1
data:
  GIT_SHA: ${GIT_SHA::8}
kind: ConfigMap
metadata:
  name: any-old-app-version
  namespace: devl

Downstreams can still get the latest value since it's in a configmap that you can refer to, manifests in git can stay DRY as originally intended... I honestly don't really want to change the way that substituteFrom works, I just think adding the GIT_SHA variable makes a lot of YAML manipulation work that would have needed to be done by as "manifest generation" no longer necessary and neatly fixes a number of use cases that were broken by #802.

[jalaziz]

In our use case, a ConfigMap containing the Git revision SHA would be perfect. We need the SHA to pass to a helm chart through values.

Since overriding values using a ConfigMap is already supported, a "managed" ConfigMap would be perfect.

We could "commit" the values back to the repo, but that's largely redundant and not actually a great experience in a monorepo because, in our case, the deployed artifacts would point to a revision that is the parent of the fully materialized and updated manifests.

[kingdonb]

That sums up why I wrote this proposal 💯 @jalaziz that is the use case in a nutshell.

[kingdonb]

So, I went and did some digging to see what would be needed to implement this proposal, and so far I am stuck. So I am going to write it out in detail in the hopes that I will get un-stuck and see a simple implementation without much change, to submit as a PR.

Here's my research:

*KustomizeGenerator and *KustomizationReconciler interfaces both call substituteVariables, here and here:

• https://github.com/fluxcd/kustomize-controller/blob/ff38a3f2cc5ba42f546bf962fbbfb1bdfccccbe1/controllers/kustomization_controller.go#L547
• https://github.com/fluxcd/kustomize-controller/blob/ff38a3f2cc5ba42f546bf962fbbfb1bdfccccbe1/controllers/kustomization_generator.go#L256

Currently substituteVariables handles first SubstituteFrom and then Substitute structs in the PostBuild spec. This would be a different substitution, we are not naming a ref, and we are not providing a variable with an override/default value as in Substitute map[string]string.

• I'm going to propose adding SubstituteFromOwnSource as an additional struct

Aside: (NB. then I will quickly put this idea aside, as I think it's overcomplicating the matter to make this behavior opt-out, makes this change big enough to no longer count as low-hanging fruit... let's NOT generalize a solution as I describe here in the aside), new list where we could name variables like FLUX_GIT_SHA that we want to opt-in and allow PostBuild to sub for us with whatever information they should contain, and in this list we can accept whatever else comes along that we think is appropriate to substitute from our own upstream source.

I would make it either a string or an array of some enum type, which FLUX_GIT_SHA is a member of the (currently non-existent) enum type. This is so that we can opt-in to the FLUX_GIT_SHA variable being substituted. If we decide this variable should always be made available when a GitRepository Source is used, then we can forget about this struct and leave the CRD spec as it already is.

(NB. again, I am explicitly not making this part of the proposal. I just want this one variable, it has specific use cases that cannot be solved without it, and since the Kustomization already knows what ref it reconciles, it should be easy to make this available at postBuild time.)

The easiest way to implement this change is to make it always-on, with no opt-out, then there is no need for any CRD change. This is a breaking change but with a limited blast radius, we can set an expectation that variables which start with FLUX_ are reserved, leaving room for growth here. I think I've seen this idea discussed before already.

So, in the code of kustomize-controller, how do we implement this? In substituteVariables we are passed in:

ctx context.Context,
	kubeClient client.Client,
	kustomization kustomizev1.Kustomization,
	res *resource.Resource

and we return *resource.Resource, error... doesn't appear to be any help here. I was hoping to find the revision.

In Kustomization we have Spec and Status only. Spec contains a ref to the source, but now this is not as easy as I guessed. We can look up a source and grab the git sha if it is available but this is bigger than the breadbox. Keep searching, the revision string is in here somewhere, it must be since Status is updated with messages like this one:

Applied revision: main/388dfa38edc2435932c2dddfa4c1c9c104965f16

I had hoped we would have the string somewhere handy that we can simply add it as a parameter to substituteVariables. Keep searching... how does Status get updated in Kustomization CR? By calling patchStatus in 5 different places,

• if the source is not found
• if the source is not ready
• if a dependedOn Kustomization is not ready
• after marking the Kustomization as progressing <-- (between here in the main Reconcile function on *KustomizationReconciler.)
• after the Kustomization has been reconciled (successfully or unsuccessfully) <-- (and here where *KustomizationReconciler's function reconcile is called, with a sourcev1.Source as a parameter. Here!

On this line, we can see where the Revision string comes from:
https://github.com/fluxcd/kustomize-controller/blob/5da1fc043db4a1dc9fd3cf824adc8841b56c2fcd/controllers/kustomization_controller.go#L398

That's as far as I can afford to spend time digging into this today. At least I have demonstrated that the data we need is in the vicinity of where we need it to go, in order to make this feature work. It should be a simple matter to demonstrate how it can work next. I am not prepared to follow up this proposal with a PR just now, but would like to let it percolate while I work on documenting other use cases of Manifest Generation that are more interesting and definitely more urgent for me, like jsonnet and cdk8s, powerful tools that can be used for "rehydrating" structures that are simplified versions of the YAML manifests.

Realistically, practically any one of these tools can do a better job than sed I've used here, in CI they will also have the git repo's revision handy and it could be rehydrated into the YAML from there, with the technique that I originally demonstrated or something nicer which I will hopefully get around to showing soon.

Doing this in CI solves another problem too... what happens if the manifest is sync'ed before the CI finishes, and the image is not ready yet?

Since this is obviously not the only time and place one might need CI to commit back to the repo, it is not urgent for me to solve this. I would love to see a PR from any new contributor to whom this is either more important or more urgent; I think this small change I propose, while limited, is "low-hanging fruit" that knocks out a few troubled use cases without compromising much. I think we should do it, but I will understand if it is not a priority for the maintainers to implement this.

It's small, and really any one of us community members could probably take it from here!

[kingdonb]

How are users suppose to prevent a commit that fails to build from reaching the cluster and crash looping with ImagePullBackOff?

Easy - if the deployment is important, then it must have MinimumReplicasAvailable set, this is a standard and best practice already. If the new pod comes up, then it can replace the old pod, and as one hopes it turns out, Deployment will update making observedGeneration >= generation, so an older pod can finally be Terminated.

There is no problem if the build fails (one could assert), it will not necessitate any SLO breach. It is still an alertable condition. Eg. someone should be notified, the build has broken, (and as I'll assert, that is another condition that should be avoided through the use of tagging and promotion strategies in important environments, through review and by policy.)

Whether this idea is acceptable for any person or not, probably depends on whether you consider a broken CI build on the branch as an alertable condition. I say it is. Of course this decision could vary from one branch to the next, just as well.

If my main branch is the deploy branch, then I will prevent the failed build from reaching the production cluster by adding merge checks and using staging and promotion strategies. If a build fails, it will not be possible to merge to a production deploy branch. Even if it did, Kubernetes stops this impacting services, through enforcing MinimumReplicasAvailable.

[jalaziz]

You can make CI build an image, tag it with the git sha, then commit the tag to the deploy branch: https://github.com/gitopsrun/webapp-frontend/blob/master/.github/workflows/staging.yaml

See the result here: gitopsrun/webapp-frontend@552e00f

This does exactly what @Fabian-K was asking for: kustomize edit set image app=<GIT-SHA>

Unlike what's proposed here, if the build fails in CI, the commit will never reach the cluster. How are users suppose to prevent a commit that fails to build from reaching the cluster and crash looping with ImagePullBackOff?

In our case, we want the feature (through ConfigMaps for use with Helm), but still want to trigger the reconciliation manually through webhooks or a different mechanism.

A failed build is fine since we wouldn't fire the event that would trigger manual reconciliation anyway.

The issue we have with updating the repo with the "built" SHA is that you can't actually go to that commit and redeploy everything (you actually need the CI committed SHA for that). It's not the end of the world, but for the case where images are tagged with the Git revision, it would be great if we didn't need the extra commit.

[kingdonb]

There are some extremely valid reasons why this proposal won't work (or why it won't – as formulated here – solve as many use cases as I think I hoped). Reason one: Flux manifests are not usually baked into your application repos, and Flux system clusters aren't usually used to deploy just a single application. Flux deploys from a single branch.

In the dev cluster I described, I want my application to deploy the latest commit on any branch. This is a "poor-man's preview environment" and I think it is an important use case. "We only have one dev environment, we don't want multiple preview envs – just follow the latest commit from any branch, I don't care if it's master, or main, or dev, or feat/some-completely-new – just wait for commits and deploy them." It's important to note here, that this is not a use case currently served by Flux v1.

In Flux v1, you have your flux repo, where all of your manifests are baked into, and you have the image automation which monitors image repos for a new image matching a filter.

Maybe you have added a subdirectory to your app repository, and maybe you are doing all of this with one repo, but the bottom line is Flux does image updates on/monitors and deploys from a single branch, (or tag, but specifically not: every branch in the whole repo, and not: whatever newest commit from a list of branches.)

So if your Flux instance is deploying from the dev branch, or deploy branch, it was never going to be able to suck up all random commits from any old branch. You must have a deploy branch, and a commit to be deployed has to go to that branch.

I still hope we can include this (opt-in pass-through of the Artifact Revision data, to meet these specific use cases) in a future version of the CRD for Kustomize Controller, but this proposal does not cover that, and what I think I've really done here, is basically knocking on the door of Preview Environments.

I know that the Flux maintainers have their eye on preview environments as a feature target already. That is a bigger proposal and I think it will subsume this one.

So, @jalaziz, point being: even if it's OK for you that a failed build that failed to push an image (or worse, pushed an image but failed CI) might still be staged erroneously in Flux, reasoning that you'll use gate triggers and you won't trigger a deployment for a failed CI result... then there is still the problem that (unless you're pushing each new commit to the deploy branch, which I think you'll agree seems inadvisable and inflexible) the commit that you want to deploy a GIT_SHA from, will usually not be the same commit that Flux is deploying manifests from.

That is why I will unfortunately not be pursuing this proposal at this time, or with this design. I've come to this conclusion after discussing it with Flux maintainers, and I hope this explanation makes sense – they have been on the project much longer and so they have a better innate sense of what designs are dead ends vs. what ideas are really going to cover more use cases than some singular niche application. I've been convinced, it is a more complicated issue than I originally thought. The solution proposed here is an incomplete stop-gap and a no-go.

I'm happy to talk it over on the CNCF slack if you want to point out something you think we may have missed. I will not forget this use case and I will continue advocating for it, since I think there is still a big gap in functionality here that needs closing. It just needs a bit more thought first, (and it should probably be considered as a subset of the preview environment feature.)

[jalaziz]

In the dev cluster I described, I want my application to deploy the latest commit on any branch. This is a "poor-man's preview environment" and I think it is an important use case. "We only have one dev environment, we don't want multiple preview envs – just follow the latest commit from any branch, I don't care if it's master, or main, or dev, or feat/some-completely-new – just wait for commits and deploy them." It's important to note here, that this is not a use case currently served by Flux v1.

This is exactly what we're looking for for part of our problem.

Will need to hop on over to Slack to see all the details and continue the discussion, but what you've described makes sense if you're using Flux in its entirety.

I think one thing that's being missed is that Flux v2 is meant to be a toolkit (at least that's what docs have lead me to believe).

We don't use deploy branches, or at least not in the way that you've described. We have a mainline branch and release branches that we manually "cut". We also use a monorepo.

The only functionality of Flux that we need currently is triggering reconciliation of a helm chart that's contained in the monorepo (alongside the code) based on an event. The event could be the successful build of a the mainline branch or the cutting of a new release branch. In either case, the event can and will be triggered via our CI tool.

Using the toolkit approach, we can piece this together with the helm-controller, the source-controller, and the notification-controller.

There seems to be two road bumps:

• For the preview case, we need access to Git revision SHA from the GitRepository API that's tracking the mainline branch. If this information was in a ConfigMap, we're good to go!
• For the release case (e.g. deploying to staging), we need to update the GitRepository ref. Flux v2 seems to already support this if you're using SemVer, but, if not, you have to manually update AFAICT. If the GitRepository ref could be pulled from a ConfigMap as well (separate feature request), then all we're left with is having to update the ConfigMap.

Ultimate, we can and will have to maintain the ConfigMap ourselves. It would seem that we could also just build our own controller to manage the resources. However, it would seem that this sort of functionality should have a place in the "toolkit" view of Flux.

Finally, I've thought about whether this violates the GitOps principles and I'd argue that it doesn't. Everything is still pulled from Git, it's just the "triggering" that is manual (for the lack of pluggable update policies). It's even nicer in some cases because the infrastructure configuration and the code are all contained in the same commit.

[Fabian-K]

Unlike what's proposed here, if the build fails in CI, the commit will never reach the cluster. How are users suppose to prevent a commit that fails to build from reaching the cluster and crash looping with ImagePullBackOff?

To prevent the "ci failed but cluster is updated" issue we do a git merge using fast forward only as a last step of the ci pipeline on the master branch to the env/dev branch and Flux is actually watching env/dev. So developer merge into master and when the ci is successful, the is automatically deployed to the dev cluster. So I guess the only difference is that we currently don´t create new commits.