-
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
More precisely extract strings for Replacements #4555
Comments
@tedtramonte: 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. |
Please read the kustomize eschewed features. We are intentionally only permitting structured edits in replacements as per the reasoning described under "unstructured edits" in our eschewed features page. I understand that it makes kustomize and replacements much less powerful in the kinds of transformations that you can do, but it is a drawback we accept to avoid unstructured edits. Your particular case is an interesting one, and I empathize with it, but your issue seems to more directly be caused by the label length limit rather than replacements. I don't think it makes sense for kustomize to add such a broad, powerful, and explicitly eschewed feature based on one use case for which there might be other options. My advice would be to truncate the SHA to 63 characters if that is sufficient for your use case, or use versioned images without a SHA. Another option you have is to write a third party extension to kustomize to do what you want, which I'd be happy to provide some more guidance on if you are interested in taking that route. |
While you're not wrong about length being the core issue, how would you propose truncating that? Replacements is hands off and automatic as part of the overlay and truncating isn't something that Kustomize or Kubernetes does. While in my case I am technically abusing the "YAML in" part of the "YAML in / YAML out" philosophy by setting the invalid label as the replacement target, there are certainly other use cases where it would be beneficial to have more freedom in extracting a string for Replacement (starting and ending characters and indexes, regex, even a string index range). To me, what WOULD be keeping with the structured edits philosophy would be empowering the Replacements feature so that valid "YAML in" doesn't accidentally become invalid "YAML out". |
@tedtramonte All are good points. You're right that truncating isn't something that we automatically do, and I agree that in the ideal case, |
It would resolve my issue certainly, but like you mentioned, it IS an error and probably should remain that way. But not every field is limited to 63 characters, so generically truncating to 63 isn't correct. # kustomization.yaml
images:
- name: mongo
newName: mongo
newTag: "5.0.5@sha256:4897d6662abfbefef888ff25d1bea5e6443501bdc299af6aa1c6944a65d1794a"
replacements:
- truncate: 63
source:
kind: Deployment
name: mongodb
fieldPath: spec.template.spec.containers.[name=mongodb].image
options:
delimiter: ":"
index: 2
targets:
- select:
kind: Deployment
name: mongodb
fieldPaths:
- metadata.labels.[app.kubernetes.io/version]
- spec.template.metadata.labels.[app.kubernetes.io/version] That would give me a label of RE: plugins, mostly just leaving my thoughts here in case anyone else stumbles on this After reading up on Kustomize plugins, that would probably be a more flexible solution, but as it stands, configuring transformer plugins (in my case specifying which manifest to get the label value from, what format is the data in, etc.) would place a large burden on the user of this hypothetical auto-version-labeler plugin. A user would have to look up and and then maintain a manifest just for adding a "standard label" which seems a bit much. Plus, the area is in active development, with a new plugin API in the works. # user has to create and maintain this in each Kustomize application
apiVersion: autoversionlabeler/v1
kind: AutoVersionLabeler
metadata:
name: notImportantHere
annotations:
config.kubernetes.io/function: |
container:
image: example.docker.com/auto-version-labeler:1.0.0
spec:
kind: Deployment
name: mongodb
# a user would probably want to specify a container within the deployment, too My issues with the above are that anyone not versed in custom Kustomize plugins isn't going to know that |
Just following up, I did end up writing a Kustomize plugin to do this. The source and docs are available at https://gitlab.com/tedtramonte/auto-version-labeler. At this time, there's some undesired behavior if the plugin is used in a In testing, listing the |
Is your feature request related to a problem? Please describe.
Following https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/ I began using the Replacements Transformer to ensure my
app.kubernetes.io/version
label matched what was in the image its container. For example, a simple Mongo setup might look like:And this works great, albeit not intuitively.
However, when my images are tagged with an sha256, things break down. Here's one non-functioning configuration:
What about using JUST the hash as the version?
Well, how about just getting the image version using @ as the delimiter?
Describe the solution you'd like
I'd like to be able to more precisely extract substrings.
Ideally I'd like to have the entire SHA256 hash as the version label, but that isn't going to happen as long as labels are limited to 63 characters. My initial reaction was to truncate the replacement to 63 characters, but Replacements is a general tool not limited to labels. Further, a 63 character hash isn't very useful as a label, anyway.
One idea is to have a starting and ending delimiter option. Example:
Of course, having just written that out, it looks awful to use.
A better option might be to allow using regex for extracting the replacement:
I'm willing to take a shot at implementing this if it's not an unreasonable addition.
The text was updated successfully, but these errors were encountered: