Check if root is given. If yes, use it as the package root. If not, go to the next step.
Check that there is path/kustomization.yaml file in the repo. If no, then simply send all *.yaml/*.yml/*.json files from that directory recursively. If yes, go to next step.
Start looking for Krmfile/Kptfile in path directory and it's parent directories to locate the root? Once the package magic file is found, the whole package is sent to the agent.
If it's not there, then simply send all *.yaml/*.yml/*.json files from that directory recursively, and log a warning.
Where
root means the top directory that is sent to the agent
magic file is one of
Krmfile or Kptfile is present, (somehow) apply setters appropriately
For Kustomize support the following configuration structure is proposed (please leave comments):
gitops:manifest_projects:-id:group1/project2paths:-glob:some/dir/*.yaml-id:group1/project1applications:# the term "application" comes from https://kubectl.docs.kubernetes.io/references/kustomize/glossary/#application-name:app1root_dir:/apps# directory which limits what's available for Kustomize to refer to.kustomization_dir:app1/overlays/prod# directory within "root_dir", where we should look for a kustomization.yaml file-name:app2root_dir:/appskustomization_dir:app2/overlays/prod
I like the applications concept and I think it could be applied generally here rather than something that is specific to supporting kustomize. Instead we could make the config options more generic and be smart about how the apply is executed based on what types of files/resources are on the specified path. For example:
Krmfile or Kptfile is present, (somehow) apply setters appropriately
otherwise, recursively apply all resources at the specified path
Such config might look like:
gitops:manifest_projects:-id:group1/project2applications:-name:something# should probably be optional and just default to `${root}/${path}`path:some/dir# equivalent to old `glob: some/dir/**/*.{yml,yaml,json}` syntaxroot:/# optional-id:group1/project1applications:-name:app1path:app1/overlays/prodroot:apps-name:app2path:app2/overlays/prodroot:apps
@ash2k also note (per my original proposal in gitlab-org/cluster-integration/gitlab-agent#124 (closed)), once you start to make certain properties optional, the usefulness of the structure as defined begins to erode. The same structure could be flattened like so (and is more readable IMHO):
I eliminated the root option in the above example because I think it has fairly low utility given that you are already in the context of a git repo, but everything still holds true with or without it. An identifier (and/or name) for each application could still be generated (following the go-getter format) as annotated.
[edit]: one other note, in (what I think is going to be) the typical case of the config and manifest project being the same, your agent config ends up being very terse and portable for cloning into another config project:
I like the applications concept and I think it could be applied generally here rather than something that is specific to supporting kustomize. Instead we could make the config options more generic and be smart about how the apply is executed based on what types of files/resources are on the specified path.
Yeah, I thought about this too. Let's try it, if we can make it work that'd be a better UX.
Krmfile or Kptfile is present, (somehow) apply setters appropriately
otherwise, recursively apply all resources at the specified path
It makes sense, apart from the setters. I think the user must have used them already and have committed the changed YAML to the repo, no? There is nothing for us to do.
gitops:applications:...
I like the above structure, it's better than what I came up with! I'd change project to project_id for consistency with our APIs.
I eliminated the root option in the above example because I think it has fairly low utility given that you are already in the context of a git repo, but everything still holds true with or without it.
Yes, we are in the context, but it's a way to limit the files that need to be downloaded. We don't clone the whole repo, we "clone" into RAM what root is set to. Then within that path (or kustomization_dir in my example) we look for the "marker file". root is optional, it is set to path if omitted (and path is set to an empty string) i.e. you cannot reference what's outside of path if you omit the root. An alternative would be to keep root as an empty string and read the whole repo, but that's inefficient as it might have a lot of unrelated files. If the user really needs that they can set root to / and that would be respected.
An identifier (and/or name) for each application could still be generated (following the go-getter format) as annotated.
Yes, but with name I wanted to give users an explicit way to define the identity of the application. Then they can keep it if they move it across agents and projects. It's like the inventory object template that's stored in the repo, but for configuration. Hm, maybe that doesn't make sense. We need something to use if the inventory template is not there, so I thought we could use agent id + this name rather than path in the repo. That would allow to decouple identity from where it's in the repo - user can move the files around, refactor the package as they wish without and "consequences". That still would not address the potential requirement to decouple from the agent id, but, hey, you can just use the inventory template object in the end =) Ok, maybe let's leave out the name for now and emphasize importance of the inventory template in the docs if the user wants to move things around.
So, here is what I'm suggesting (minor changes to your proposal):
gitops:applications:-# no project_id specified, defaults to the configuration project id# no root specified, rewritten into root="some/dir" and path=""path:some/dir# path within the repo where we look for "magic files"-project:group1/project1# other projectroot:appspath:app1/overlays/prod-project:group1/project1root:appspath:app2/overlays/prod
@ash2k I'm totally happy with this overall and the following is just nitpicky stuff:
It makes sense, apart from the setters. I think the user must have used them already and have committed the changed YAML to the repo, no? There is nothing for us to do.
You are correct that some setters are going to be applied before committing to the repo. I had a few other use cases in mind here (the last one being the most needed from our perspective):
"runtime" information regarding the config project and/or agent applying the manifests
this allows you to inject some context into manifest projects you are referencing remotely
non-sensitive setter values that you want to be different between agents (staging and production for example) without having to checkin separate copies of the manifests
a means of secret injection from (what we currently call) "protected" CI variables
... or some other future iteration of that concept which could be scoped by agent name instead of environment name
I like the above structure, it's better than what I came up with! I'd change project to project_id for consistency with our APIs.
No strong opinion here, but as a user I tend to think of the project_id as the numeric id.
Yes, we are in the context, but it's a way to limit the files that need to be downloaded. We don't clone the whole repo, we "clone" into RAM what root is set to.
Yea I thought about that, but it bugs me that there is no UX/feature related reason for root to be specified. It also leaks an implementation detail about kustomize specifically that I don't have any reason to care about as a user.
I also think there are other ways we could be smart without any hints from the user:
given path: apps/app2/overlays/prod you have to start by shallow-cloning that path to know if it even has a kustomization.yaml
if it does not or there are no sibling/parent references in the kustomization.yaml you're done
if there are, you can recursively expand that, shallow-cloning the paths you need
recursive expansion could be cached and shared between all applications that share the same project for a given sync (commit)
Another thing to note here is that if we start supporting some kind of starlark scripting (gitlab-org/cluster-integration/gitlab-agent#103 (closed)) we are probably going to want some kind of caching layer like (4) for that anyway.
So, I think we could get by not having root at all for now and optimize later. If you think we need some level of optimization now without being smart I think you'd want the spec to look like:
I had a few other use cases in mind here (the last one being the most needed from our perspective)
These are all great use cases. If you could create a separate issue for them that'd be great (just trying to not lose this but also limit the scope here otherwise it's too much). We may have to set some setters, but that should also happen in the agent, not on the server.
it bugs me that there is no UX/feature related reason for root to be specified. It also leaks an implementation detail about kustomize specifically that I don't have any reason to care about as a user.
I also think there are other ways we could be smart without any hints from the user:
I think this is a great point! I agree that the user shouldn't need to care, but I also really don't want to parse any files on the server-side in kas:
It just creates security consequences (user fuzzes our YAML/Kustomize parser with their input and can crash kas if there is a bug, for example).
Creates more load on the server.
kas becomes dependent on the Kustomize's kustomization.yaml file format, which may change over time.
Complicates logic - "send all files in dir ROOT to the agent" vs "explore files and their contents starting from directory path".
Hm, another idea - what if we ask for path only and then:
Check that there is path/kustomization.yaml file in the repo. If no, then simply send all *.yaml/*.yml/*.json files from that directory recursively. If yes, go to next step.
Start looking for Krmfile/Kptfile in path directory and it's parent directories to locate the root? This can be done cheaply without downloading anything. Once the package magic file is found, the whole package is sent to the agent. If it's not there, we can send the whole repo, but log a warning. Sending the whole repo should not be too bad as we have limits in place and also we should do agent-side caching anyway. However, I'd rather stop processing here and ask the user to put the file where it should be. I'm worried that on GitLab.com this would be extra load that can be avoided and that those extra unneeded files will take up extra RAM on the agent side. Just wasted resources. I think this is justifiable because it's in the user's interest.
Another thing to note here is that if we start supporting some kind of starlark scripting (gitlab-org/cluster-integration/gitlab-agent#103 (closed)) we are probably going to want some kind of caching layer like (4) for that anyway.
File contents caching (git object OID/sha -> contents) should be done on the agent side. Agent should send kas OIDs it has so that kas can skip sending the contents of those objects.
but I also really don't want to parse any files on the server-side in kas
Ah, yea I forgot all the communication with gitaly is done by kas and not agentk, so I see where you're coming from now.
Hm, another idea - what if we ask for path only and then:
Start looking for Krmfile/Kptfile in path directory and it's parent directories to locate the root? This can be done cheaply without downloading anything. Once the package magic file is found, the whole package is sent to the agent.
This seems like a good idea. I wish there were better standards/guidance on when/where these "magic" package files should be present. As you know, both Krmfile and Kptfile are completely optional. Nevertheless, I think this strikes a good balance between ease of implementation and functionality. I think we could just provide doc/guidance around this, but a couple edge cases to be aware of:
by not analyzing the contents of <path>/kustomization.yaml we have to assume the worst case
i.e. even though my kustomization.yaml only has local path references, because I have no Krmfile we end up cloning / when all we needed was <path>
from your original example, it would not be "correct" necessarily, but not unreasonable for the user to have package files at apps/app1/ and apps/app2/ rather than apps/Krmfile
similarly they could have both apps/app1/Krmfile and apps/Krmfile, so we need to traverse all the way up the tree to find the furthest ancestor (vs bailing after the first one)
... but traversing all the way up isn't necessarily required, just safe
still far from ideal in terms of efficiency. in order to install app1 we end up cloning apps/** when all we actually need is:
apps/app1/overlays/prod
apps/app1/base
apps/shared
Like I said though, I think these are all acceptable tradeoffs and we can always improve the granularity in the future.
[edit]: I think maybe it makes sense to flip the logic around to say if we detect no "root" Krmfile/Kptfile we just send <path> so that we can be a bit more optimistic in that first case.
@ash2k@marshall007 I updated the description with your shared proposal. Let me know if you'd like to change something, or if you have a proposal for the iteration.
My 2nd step would be "Assume that there is path/kustomization.yaml file in the repo. Simply send all *.yaml/*.yml/*.json files from that directory recursively."
@nagyv-gitlab@marshall007 I've re-read the conversation above and the proposal in the issue description makes sense to me. I think what we haven't worked out yet (only touched upon that above) is how to "set the setters".
➜ ~ kustomize version{Version:kustomize/v4.4.1 GitCommit:b2d65ddc98e09187a8e38adc27c30bab078c1dbf BuildDate:2021-11-11T23:27:14Z GoOs:darwin GoArch:amd64}➜ ~ kustomize cfg --helpCommands for reading and writing configuration.Usage: kustomize cfg [command]Available Commands:cat[Alpha] Print Resource Config from a local directory. count [Alpha] Count Resources Config from a local directory.grep[Alpha] Search for matching Resources in a directory or from stdin tree [Alpha] Display Resource structure from a directory or stdin.
kpt does in-place hydration - it mutates the package contents. The user should be able to mutate the manifests and any other files and then commit them into the repo. That also means "nothing to do here" for us.
If someone wants to change a manifest depending on some factor for a particular agent, they just create an overlay with needed mutations for that specific agent(s) and configure it to apply that overlay.
However, I suggest we at least think about these two things, before rushing to implement this:
@ash2k yea I completely agree. I don't think it makes sense to support setters anymore. A lot has changed since we had this discussion originally.
In terms of where to go from here, I was just looking at the Flux architecture/API and it makes a ton of sense to me: https://fluxcd.io/docs/components/
It consists of several independent controllers that, when composed together, form an end-to-end GitOps "pipeline". I find this architecture very appealing because components are maintained independently. Rather than hardcoding a bunch of behaviors into agentk and/or being super opinionated about how agent config/manifest projects work, we could just focus on providing good abstractions that enable users to do whatever they want.
In other words, agentk would basically become a "controller of controllers" and its responsibility would be to manage the lifecycle of itself and all the other components. That way the user doesn't have to deploy new versions of the agent explicitly to get new features, they just get rolled out automatically when the version of kas they are connected to changes.
I wrote and then deleted a comment as I think it's off topic here. This issue is quite verbose already, I suggest to create a separate issue for this discussion if necessary.
@ash2k agreed, let me know what your thoughts are here: #350697
I don't recall where, but several months ago you were making the argument that we should keep the Agent relatively tech-agnostic. I'm much more inline with that philosophy now. The caveat being that we still implement similar functionality, in separate controllers, with well-defined APIs in the form of CRDs.
The latest description says this with regards to helm: Chart.yaml is present, use helm; however, this seems to exclude usage of a chart museum, and only allows for locally cloned charts. That feels like a severely limiting factor, unless there is some syntax that can be put in a Chart.yaml to refer to a museum chart (specifically, proxied through an Artifactory virtual repo for on-premises use behind a proxy)?
In my opinion, most helm users would be relying on museum charts, and not locally developed/cloned charts, which this feature description currently only supports.
Would using the chart as a dependency of a new chart, not cause Values.yaml files have to be rewritten to include an extra top level node, which could result in bugs?
Perhaps another "magic file" like helmrelease.yaml, with a field specifying the repository and chart and values files and values? There seem to be a lot of necessary knobs to tweak for a Helm release, looking at FluxV2 at least: https://fluxcd.io/docs/components/helm/helmreleases/, so it seems a magic file would be required, as these options cannot be specified in a chart.yaml file.
In addition, there would be a helm repo add command prior to the helm install command. We are an on-prem customer, and our things have to be proxied through Artifactory.
@brett_jacobson Thanks for the details. I was looking at the Helm Controller docs too. I am thinking how could we simplify the configuration required by Flux simply because we know where the code comes from.
For Flux you need to define:
Helm chart
Helm repository
Helm release
including ConfigMaps / Secrets for the values
We likely need all the references they use, but might be able to provide conventions for pulling the values together instead of defining an intermediary ConfigMap. Merging cluster-side Secret-s would still require access to the cluster.
Taking all these into account, I wonder if integration with the Helm and Kustomize Controllers might be the best approach.
We don't have an answer for secrets. We could add Secrets to the list of objects, but server-side we don't have a mechanism to get secrets for a particular repository. We could rely on CI variables, but not all of them should be injected. Also, some secrets need to go into one container and some into another. Some might need to be used as pull secrets too. We need to come up with a list of use cases and requirements and a plan.
@ash2k Couldn't we solve the secrets question by assuming our users know what they do and we provide one type of recommendation for the users who don't know it? This recommendation could be either external secrets or sealed secrets or SOPS
I prefer the latter two. They have a rather minimal and platform-agnostic approach, that uses the git repo to store encrypted secrets.
Helm needs to support secrets within itself because it needs to describe all the related resources. Btw, Helm Secrets are built on top of SOPS too.
@nagyv-gitlab This may not work in 100% cases, but is probably good enough for a lot of users. We need user stories and use cases that we want to support. Then we can determine what would work.
Kustomize supports KRM functions which must be implemented as a container. Kustomize just does docker run ... to execute a function. We don't have docker inside of the agentk container, and even if we did, it wouldn't work. We need to try to solve this problem, otherwise Kustomize packages will be of limited usability.
@ash2k The more I think about the templating question, the more I am convinced that the best would be to have an execution engine where we can push jobs and receive the resulting hydrated manifests as an artifact. Couldn't we re-use the Runners for the templating support? WDYT?
Today, there is a feasible workaround to define CI jobs to run helm template ... or kustomize build ... followed by a git commit and git push. Writing out all this logic is really annoying, but it works. What if we just provide syntactic sugar and some integrations with the Runner, so there is no need to git commit and git push and one can pipe multiple "jobs" one after the other?
The more I think about the templating question, the more I am convinced that the best would be to have an execution engine where we can push jobs and receive the resulting hydrated manifests as an artifact.
Couldn't we re-use the Runners for the templating support?
Automatable DevOps suggests to have a generic execution API - #328489. I think (but I'm not 100% sure!) this, unfortunately, would not always work for KRM functions because docker-in-docker is not always possible. E.g. if Runner runs on top of Kubernetes and not a VM, then it cannot start a container. This needs to be investigated properly.
However, we don't need to reuse Runner for that, agentk can just spawn a Pod by itself. The problem is capturing stdout and stderr of a container of that Pod. We need to wrap the KRM function container in code that would invoke it and capture stdout and stderr. And that's what we cannot do with a Pod. Maybe we can use logs for that? Not sure, may be possible but messy. Needs a bit of investigation.
Get the entrypoint/cmd of the function container (i.e. binary/command inside of the container that needs to be run). We could use generic registry API to fetch metadata only, not the whole image. Authn needs to be taken care of.
Mount a special program into the container (with correct CPU architecture).
Pass the entrypoint/cmd to the program using env vars/volume. Run the special program in a Pod with that container as the image.
The program can run the entrypoint/cmd function, pipe the input into stdin, capture stdout and stderr, exit code. Then upload the output somewhere.
The latter might contain hooks, secrets and other features that can not be inflated. We should be able to support both. The HelmChartInflationGenerator could provide support for the first use case and for all the kustomize-only use cases.
Current solution for this problem: No solution available. Customer wants to use KAS to support of templating tools (kustomize, kpt, helm)
How important to customer: Very important, as requested by the customer in a dedicated email to TAM in early June. Because customer uses HELM for deploying most of its applications, including GitLab itself.