-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Deprecate vars #4049
Comments
@natasha41575: 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. |
@natasha41575 To make something similar to
Basically what this solution does - in case muliref object is set all values extracted there are passed to go-template as values. go template engine uses that as .Values and it has to produce resulting string that will go to target. Target in this example is the same field as source[0]. Go-template engine had Sprig-functions included.
This approach in practice demonstrated a lot of flexibility - it's possible to concat string, build string with patterns, substitute part of it and many other things, because go template is basically a language and that allows alot.. I guess it's much more flexible than approach with delimiter. From implementation standpoint it was really easy to implement it: |
Per offline discussion, we keep var in kustomize for one year according to the kubernetes one year deprecating policy. When var is detected in kustomization.yaml, kustomize warns users about the deprecation date, and request them to migrate to the replacements field. |
The question that comes to mind for me is how exactly we want to do this deprecation. Since this is a field in the Kustomization API (as opposed to a CLI cmd/flag), if we follow the Kubernetes API backwards compatibility policy, we shouldn't ever remove it from v1beta1 Kustomization; instead, we should cut v1 with vars etc. already removed, and have the CLI support both v1beta1 and v1 for the year deprecation period. This would also mean a single migration guide for all the deprecated features, and support in the So overall the process would be:
* A huge downside to that is that it requires all users to take some action, not just the ones affected by a deprecation. Perhaps before emitting deprecation warnings, we could also implement a silent conversion feature to enable v1beta1 support indefinitely if and only if the instance is using fields that exist in both versions or have exact equivalents (e.g. bases->resources but not vars->replacements). In other words, if and only if carrying that support will not incur technical debt / inability to remove unsupported features from the codebase. WDYT? If we go with that plan, we can start it any time IMO--the first step is mostly a matter of confidence in the feature set we want v1 to retain, and of figuring out how to do this cleanly in the codebase. Experience adding Composition (which I'm starting now) may be informative for the latter. Regardless, you're right that the long deprecation period leaves us plenty of time to implement more enhancements to replacements as required. Incidentally, several of the asks seem to be for unstructured edits / variable substitution, which we will definitely not support. For the cases where the problem is essentially problematically structured CRD fields, the plugin graduation story may also play a big role in preparedness for feature removal. |
The overall process you've proposed LGTM. We can start looking at some of the issues in https://github.com/kubernetes-sigs/kustomize/projects/12 to prepare for a v1 Kustomization.
SGTM. So long as all the fields are supported, I see no issue with converting v1beta1 to v1 silently. |
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
/remove-lifecycle stale |
/lifecycle frozen |
Any chance of multiref becoming a core API of kustomize/replacements? Without this capability, I'm afraid deprecating vars breaks the templating capabilities of native kustomize and requires using helmCharts in kustomization abuse to introduce the same capability in a non-native kustomize approach. |
That's the point. Kustomize was built around the idea of template-free YAML - this is the first line of the README. There are lots of reasons that we believe that templating is not great, you can read about them here under "unstructured edits": https://kubectl.docs.kubernetes.io/faq/kustomize/eschewedfeatures/ and many more details about the pitfalls of parameterization in Brian Grant's Declarative Management Application proposal. Introducing That being said, we understand that users are already using vars, and we are trying to come up with reasonable migration solutions for those users. |
Using replacements with multiref enables structured parsing of arbitrary fields and joining them together, this would largely resolve issues with the deprecation of vars, and was entirely ignored by your response. |
Apologies for not addressing it in my response, I did read it and we are still thinking about it. The original proposal above has some syntax with templating/regex that we would most likely avoid but allowing the source to come from multiple places and concatenate them together is a very common use case that I myself have dealt with as well. In my own work, what I've done for this is write a KRM function (I use the kpt starlark function) to generate the desired string, pulling it from multiple sources and concatenating it, and then using Replacements is already quite verbose and has complex syntax, and I have concerns that introducing multiref natively in replacements will exacerbate its complexity. |
Thanks for addressing it. It is a complex topic, but it's a very common use-case and a "standard" approach to resolving the issue would benefit the community. Unfortunately, leveraging KRM/kpt expands the trusted deployment code base, and for auditable environments that becomes impractical due to review processes involved. That overhead can be so significant that it is preferable to wait for a new
Yes, the implementation is complex/verbose, however I think replacements is a great swiss-army knife of kustomize. =========================== My most common use cases where I need these capabilities and kustomize flounders badly, is with generating strings of formats such as:
Below is how I use configMaps to generate "structured variables", however schema prefixed strings like bucket names and connection strings with their non-uniform delimiters can't work with this approach. kind: Component
configMapGenerator:
- name: aws
# behavior: merge
# arn:partition:service:region:account:resource
options:
disableNameSuffixHash: true
literals:
- partition=aws
- region=us-east-1
- account=1234567890
- iam.resource_path=$(program)/k8s
- iam.permitted_arn=arn:[^:]*:iam::[^:]*:role
- ecr_url=1234567890.dkr.ecr.us-east-1.amazonaws.com
- REF_AWS_REGION=region #literal source in replacements would remove the need for this reference to grab the region
# valuesInline:
# serviceAccount:
# annotations:
# eks.amazonaws.com/role-arn: arn:$(aws.partition):iam::$(aws.account):role/$(aws.iam.resource_path)/$(cluster)/$(namespace)/role
# deployment:
# annotations:
# iam.amazonaws.com/role: $(aws.iam.resource_path)/$(cluster)/$(namespace)/role
replacements:
- source:
kind: ConfigMap
name: aws
fieldPath: "data.[iam.resource_path]"
targets:
- options: { delimiter: "/", index: 1 }
select: { kind: Namespace }
fieldPaths: [ "metadata.annotations.[iam.amazonaws.com/permitted]" ]
- options: { delimiter: "/", index: 1 }
select: { kind: ServiceAccount }
fieldPaths: [ "metadata.annotations.[eks.amazonaws.com/role-arn]" ]
- options: { delimiter: "/", index: 0 }
select: { kind: Pod }
fieldPaths: [ "metadata.annotations.[iam.amazonaws.com/role]" ]
- options: { delimiter: "/", index: 0 }
select: { kind: Deployment }
fieldPaths: [ "spec.template.spec.containers.*.metadata.annotations.[iam.amazonaws.com/role]" ]
- options: { delimiter: "/", index: 0 }
select: { kind: StatefulSet }
fieldPaths: [ "spec.template.spec.containers.*.metadata.annotations.[iam.amazonaws.com/role]" ]
- source:
kind: ConfigMap
name: aws
fieldPath: "data.[iam.permitted_arn]"
targets:
- options: { delimiter: "/", index: 0 }
select: { kind: Namespace }
fieldPaths: [ "metadata.annotations.[iam.amazonaws.com/permitted]" ]
- source:
kind: ConfigMap
name: aws
fieldPath: data.partition
targets:
- options: { delimiter: ":", index: 1 }
select: { kind: ServiceAccount }
fieldPaths: [ "metadata.annotations.[eks.amazonaws.com/role-arn]" ]
- source:
kind: ConfigMap
name: aws
fieldPath: data.account
targets:
- options: { delimiter: ":", index: 4 }
select: { kind: ServiceAccount }
fieldPaths: [ "metadata.annotations.[eks.amazonaws.com/role-arn]" ]
- source:
kind: ConfigMap
name: aws
targets:
- options: { create: true }
select:
kind: Deployment
fieldPaths:
- spec.template.spec.containers.[name=controller].env.[name=AWS_REGION].valueFrom.configMapKeyRef.name
- source: # literal here would be super helpful to jam in `AWS_REGION` into the key, but currently requires a dummy configmap value to implement
kind: ConfigMap
name: aws
fieldPath: data.REF_AWS_REGION
targets:
- options: { create: true }
select:
kind: Deployment
fieldPaths:
- spec.template.spec.containers.[name=controller].env.[name=AWS_REGION].valueFrom.configMapKeyRef.key |
I'd like to transition from vars to replacements and get rid of the deprecation warning..and it works in all but one case where I am unable to substitute a var that is part of an env var name that is set in my Deployment conf:
I am using this patch file:
But I get this error on an apply:
I am trying to make this replacement in my patch file from a I'd like to avoid having to add this Using replacements for variable substitution in other strings is working for other value-type paths, but not this for some reason. Is it because it is a key name? |
I'm trying to do the same as you. As far as I understood, we have to use
Then you have to replace the object under New |
More discussion here: #5046 |
The new replacements field is intended to be the replacement for the vars feature. Now that its implementation is complete, we should look into deprecating vars.
Some outstanding migration issues:
#3978
#4012
#4337
#4359
These issues have the same underlying problem that users are adding vars in strings where the var is not delimited, e.g.
Host(www-$(NAMESPACE).example.com)
.We should also take a look at use cases such as #4080 where the user wants to replace a string in an array without having to rely on the order of items in the array.
Generators for service accounts have been proposed as the solution for the first issue, but does not easily resolve the second.
options.prefix
andoptions.suffix
have been proposed for the third (more details in the issue).Additionally, #4120 requests a way to replace a template variable.
The text was updated successfully, but these errors were encountered: