-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
ConfigMapGenerator should not load values from the build environment #4731
Comments
@KnVerey: This issue is currently awaiting triage. SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Hi @KnVerey, thanks for writing this up! I am the documentor. Frankly I think it's a bad feature to eschew, and leaving in the ability to fetch manifests from remote targets while pulling this out feels like just making my life as a user hard. Kustomize is currently a single, fully usable tool for manifests. If we remove this, we will have to add another one on top of it. I'm totally sick of that. Judging by the number of issues I responded to, which resulted in me documenting it, other users don't want you to eschew this feature either. The feature has been critical several times over the last couple years. The workaround proposed requires CD to know both the env var name, the configmap name, and the folder the configmap is in in the CD pipeline definition. This means that the CD pipeline must be custom for the particular project, which feels ridiculous and exactly the problem that skaffold + kustomize solve by allowing me to declare that in yaml next to the code instead. So either we need a hook system or another system to communicate into kustomize. This one works fine, has no unexpected consequences (you can still set to empty by including the I'd like to hear stronger reasons for removing it than "we didn't think it was a good idea 4 years ago". I totally understand the stance "we don't want to be a templating engine" but replacing this interface with calls to the CLI that take two pieces of info the caller doesn't have is not great - nor is suggesting users write a transformer to run A reasonable compromise would perhaps be a My 2c as a long-time user and fan currently running kustomize+skaffold as CD for ~25 services. Here's another user's perspective, which I pretty much agree with. p.s. apologies for not tagging any maintainers on the docs PR. Perhaps you want to add an cc @justinsb - this is the one we were talking about at kubecon and also 👋 use caseHere is the use case I originally dug it out for - extracting the build tag to provide to an operator which spawns more pods at the same tag, but needs the tag in a config file: - name: Extract skaffold image tags to the environment
# some things need to know their tag;
# this let's kustomize or skaffold populate it from the env
continue-on-error: true # continue if no build artifacts are found in skaffold.yaml
run: |
skaffold build -q --dry-run | \
jq -r '.builds | map(.tag |= split(":"))[0] |"BUILD_SKAFFOLD_TAG="+.tag[1]' >> $GITHUB_ENV |
Just to be clear, I don't think the environment fallthrough is a good way - but |
This change is going to be a massive issue in our organization. We've considered it a feature for a long time (no idea how we found it originally though), and rely on it for templating ingress URLs, since most of our testing environments are generated on-demand. It's been very handy, since at least for us, the namespace and the cluster host has many times been necessary during manifest rendition. Basically, a developer requests a testing environment. A namespace is generated on kubernetes with a random namespace name. That name is then part of the URL (e.g. I could image a command I agree pretty much with @afirth on this topic, and really hope you reconsider this |
In our project we use this feature to inject credentials into ConfigMaps, using We also hope you reconsider this. |
I can understand your choice for ConfigMapGenerator, but this warning is also for secretGenerator. Today, our secrets are stored in Gitlab CI variables and are masked. Edit kustomize file to set these secrets variables is not a good way. |
Sad to see this |
Closed by #4957 |
I am sad to see that. Despite many user comments arguing this feature interest you remove it. In addition, there no response to handle users needs. |
I can't believe this happened in an OPEN SOURCE community? No matter this is a bug or a feature, given so many users have already relied on this behavior even in many production environments (including us), finding a way that's backward compatible or an alternative (apparently kustomize edit is not) would be a better approach, |
@KnVerey, can you share with us any specific reasons for the removal of this feature, besides alignment with Kustomize's core principles? Many organizations that use CI for deployment rely on this feature for their Kustomize usage, and without it, the use of Kustomize would be limited as the only alternative would be using sed. It would be great if you could consider the feedback from the community before making such a decision based on religious reasons. |
Thank you to everyone on this issue for taking the time to provide your feedback. Rest assured that we did read and consider it, but at the end of the day we still felt removal was the right decision for Kustomize. As a long-standing behavior, it is not surprising that some folks have found ways in which it is useful in their workflows–this speaks to the truth of Hyrum's law. This is why we put the behavior through a noisy (via logging) deprecation process and considered its removal a breaking change, even though it was never intended and on the contrary was something our own docs promise Kustomize will never do. Our guiding principles aren't just dogma for its own sake–they're promises we make to our users about how Kustomize works. They define what distinguishes it among the vast array of options in the configuration management landscape, and they help us keep making a great tool with a consistent vision and user experience across time and leadership changes. We regularly get feature requests to reverse every single one of these principles, and I'm sure if we accommodated them, many users would find those changes useful as well. However, it would likely break workflows for those taking advantage of the distinguishing features Kustomize was designed to provide, and it would certainly degrade Kustomize's user experience in the long run. In this case, we're mostly talking about the "no build side-effects" principle, which is a promise to our users regarding the stability of the output of With plugins, it should be possible to write a custom generator that replicates the previous behavior exactly. Our dream for the KRM Functions Registry project is that it will provide a place for community members to share functionality like this that is useful to many but is out of scope for Kustomize core for one reason or another. That project is currently owned by the same over-stretched people as Kustomize, and as such has not made progress in a while. If someone on this thread is interested in picking that up, we would love to support that. In the meantime, plugins can be shared from any repo. Kustomize's extension system is an investment area on our roadmap, and we would be more than happy to receive bug reports and PRs to fix any obstacles faced in doing this. In case someone creates such a plugin or has another constructive idea to share, I will keep this issue unlocked for that purpose. Please realize that the Kustomize team is a tiny group of real human beings, and comments maligning our intentions are hurtful and discouraging. Although we can never make everyone happy, we work very hard to make Kustomize a great tool both now and in the long term. If you would like to have more insight into and influence on the direction of the project, we are always looking for more contributors with the bandwidth to grow into maintainership roles! |
Thank you for taking the time to communicate the team's reasons and thought process, @KnVerey . It's funny you bring up Hyrum's law, as I was considering citing it myself in this discussion. Because there isn't really another way in Kustomize to replicate the removed behavior. Back when I started using Kustomize, the code's comments around this behavior mentioned it explicitly, so i always considered it a "known hack". I would have preferred that a stable alternative was present before it was removed. Currently, the company I work at has really bent-over backwards trying to come up with all kinds of hacky solutions, just so that unsuspecting developers wouldn't get caught in a snag when they updated their local Kustomize installations. While I'd love for every developer to be up-to-speed with every tool we use, the reality is quite different. While The KRM Function Registry would be a great addition to Kustomize, but as you say, the project hasn't been able to move forward in a while, so we can't rely on it either. We ended up relying on pre-render hooks in Skaffold to pull env variables and create a configmap, and then use vars (even though it is deprecated too... one problem at a time I guess). I see the vision Kustomize has, and mostly I agree. Excessive templating is a pain to deal with, and thus I still think Kustomize is a better and easier tool to work with than Helm. But reality keeps showing up, and I often encounter a few cases where some templating is needed. My dream would be for Kustomize to adopt something similar to what Terraform does with variables. Explicitly declared variables, that are easy to see and work with. The Terraform CLI can even tell you what input variables are required. To me, it is the best middle ground I've encountered. Thank you and the entire team for your hard work and your communication. Kustomize is a great tool, and I look forward to keep using it for a long time. I might not agree wholeheartedly with the vision, but Kustomize is still the best alternative out there. |
@ the rest of you I hope this helps, I believe it duplicates the old functionality in a pretty generic way # replace all instances of "FOO=" in files ending in .env with "FOO=<value of $FOO>"
perl -pi -e 's/^(\w+)=$/$1=$ENV{$1}/g' `find ./ -name *.env` those of you using this to inject secrets, please consider using something like external-secrets-operator instead - hydrating your manifests with secrets closes a lot of doors. @KnVerey thank you very much for all your hard work and for your thoughtful response. Like others, the main use case for us is populating ingress resources, something like this: - source:
fieldPath: data.hostname
kind: ConfigMap
name: environment
targets:
- fieldPaths:
- spec.tls.0.hosts.0
- spec.rules.0.host
select:
kind: Ingress
name: echo-server |
Given the fact that many have voiced the use of this 'previously considered a feature', that there is no replacement functionality, and that it had existed for some time, I think it would have been prudent to take a bit more time before ripping it out and at least consider offering an actual solution rather than just tossing it over a fence in the name of dogma. |
v1.27.1 No CLI change. v1.27.0 API Change * Adds feature gate NodeLogQuery which provides cluster administrators with a streaming view of logs using kubectl without them having to implement a client side reader or logging into the node. Feature * Added "general", "baseline", and "restricted" debugging profiles for kubectl debug. * Added "netadmin" debugging profiles for kubectl debug. * Added --output plaintext-openapiv2 argument to kubectl explain to use old openapiv2 explain implementation. * Added e2e tests for kubectl --subresource for beta graduation * Changed kubectl --subresource flag to beta (#116595, @MadhavJivrajani) * Enable external plugins can be used as subcommands for kubectl create command if subcommand does not exist as builtin only when KUBECTL_ENABLE_CMD_SHADOW environment variable is exported. * Kubectl will now display SeccompProfile for pods, containers and ephemeral containers, if values were set. * Kubectl: added e2e test for default container annotation * Made kubectl-convert binary linking static (also affects the deb and rpm packages). * Promoted whoami kubectl command. * Since Kubernetes v1.5, kubectl apply has had an alpha-stage --prune flag to support deleting previously applied objects that have been removed from the input manifest. This feature has remained in alpha ever since due to performance and correctness issues inherent in its design. This PR exposes a second, independent pruning alpha powered by a new standard named ApplySets. An ApplySet is a server-side object (by default, a Secret; ConfigMaps are also allowed) that kubectl can use to accurately and efficiently track set membership across apply operations. The format used for ApplySet is set out in KEP 3659 as a low-level specification. Other tools in the ecosystem can also build on this specification for improved interoperability. To try the ApplySet-based pruning alpha, set KUBECTL_APPLYSET=true and use the flags --prune --applyset=secret-name with kubectl apply. * Switched kubectl explain to use OpenAPIV3 information published by the server. OpenAPIV2 backend can still be used with the --output plaintext-openapiv2 argument * Upgrades functionality of kubectl kustomize as described at https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.0.0 and https://github.com/kubernetes-sigs/kustomize/releases/tag/kustomize%2Fv5.0.1. This is a new major release of kustomize, so there are a few backwards-incompatible changes, most of which are rare use cases, bug fixes with side effects, or things that have been deprecated for multiple releases already: - kubernetes-sigs/kustomize#4911: Drop support for a very old, legacy style of patches. patches used to be allowed to be used as an alias for patchesStrategicMerge in kustomize v3. You now have to use patchesStrategicMerge explicitly, or update to the new syntax supported by patches. See examples in the PR description of kubernetes-sigs/kustomize#4911. - kubernetes-sigs/kustomize#4731: Remove a potential build-time side-effect in ConfigMapGenerator and SecretGenerator, which loaded values from the local environment under some circumstances, breaking kustomize build's side-effect-free promise. While this behavior was never intended, we deprecated it and are announcing it as a breaking change since it existed for a long time. See also the Eschewed Features documentation. - kubernetes-sigs/kustomize#4985: If you previously included .git in an AWS or Azure URL, we will no longer automatically remove that suffix. You may need to add an extra / to replace the .git for the URL to properly resolve. - kubernetes-sigs/kustomize#4954: Drop support for using gh: as a host (e.g. gh:kubernetes-sigs/kustomize). We were unable to find any usage of or basis for this and believe it may have been targeting a custom gitconfig shorthand syntax. * [alpha: kubectl apply --prune --applyset] Enabled certain custom resources (CRs) to be used as ApplySet parent objects. To enable this for a given CR, apply the label applyset.kubernetes.io/is-parent-type: true to the CustomResourceDefinition (CRD) that defines it. * kubectl now uses HorizontalPodAutoscaler v2 by default. Documentation * kubectl create rolebinding -h Bug or Regression * Added (dry run) and (server dry run) suffixes to kubectl scale command when dry-run is passed * Changed the error message of kubectl rollout restart when subsequent kubectl rollout restart commands are executed within a second * Changed the error message to cannot exec into multiple objects at a time when file passed to kubectl exec contains multiple resources * Kubectl: enabled usage of label selector for filtering out resources when pruning for kubectl diff * kubectl port-forward now exits with exit code 1 when remote connection is lost
No worries, just use helm, argo or sed/bash - These tools have this feature because it is needed. Whoever eschewed the academic edict to shun this feature may have been right when this was a virgin opensource hobby project but it is 2023, all other pipeline/manifest tools have the ability to accept env or cli parameter's, hoping you can stay current. Thanks for your good work. |
It recently came to @natasha41575 and my attention that the
configMapGenerator
has a bug where it will load values from the environment when a line in an env file has a key but no value. This bug dates back to the origins of Kustomize, when some of the original code was copied out of kubectl, and this undesirable behaviour (for kustomize, not kubectl) went unnoticed in the adaptation.This behaviour is a clear violation of one of Kustomize's core principles, as documented in our Eschewed Features list:
Very unfortunately, someone not only discovered this bug, but made a PR to document it as a feature on kubernetes.io: kubernetes/website#30348. Unfortunately no Kustomize maintainers were tagged on the PR, and it has been live since January 2022. 😞
As stated in Kustomize documentation screenshot above, any need for environment reading should be handled as part of a pre-build workflow. We can consider adding a
kustomize edit
subcommand for this specifically if there is demand from those currently relying on the bug. For example, it could look likekustomize edit add configmap my-configmap --from-env-file=env/path.env --from-env=ONE,TWO
.Since this behaviour has been around unnoticed by us for so long, our plan is to to deprecate it and remove it with a major version bump rather than treating it like a bug fix. We are reverting the docs change in kubernetes/website#35522 and emitting a warning to affected users in #4730. The final step is to actually fix the behaviour in the next major release, i.e. Kustomize v5.
The text was updated successfully, but these errors were encountered: