-
Notifications
You must be signed in to change notification settings - Fork 23
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
Prevent requiring a "metadata.name" when using include/exclude kind flags #88
Comments
Well, I guess the workaround is to use I can accept that. Most tools are fine with that present. There's still the difference in behavior with the include/exclude flags. |
For those that aren't aware. Lists are nice because you can use YAML anchors/aliases across resources because the List is processed as a single YAML document. |
Hey @matthewhembree! Would you mind helping me understand your issue? I see you're rendering a Helm chart which you then pass to I assume this Helm chart cannot be sent as-is to Kubernetes, though, right? It's not possible to do this: helm template . | kubectl apply -f - In the issue you link, someone asked to have lists parsed. While there are lists in the API, they're internal to Kubernetes and not truly exposed unless it's to render out a more meaningful output than just an array of JSON data. In fact, For the second part of it, why is Still, I would be happy to support your use case scenario, but we have to do it in a way that a) honours, first and foremost, Kubernetes native behaviour; and b) ensures it doesn't make assumptions of future implementations (because it would mean we would have a snowflake behaviour that people must know and acknowledge which we might have to tear down with a breaking change in the future if our implementation skews away from whatever the Kubernetes org decides). I'm truly curious though: is your Helm chart not supposed to be installable in a cluster? Say, if you were to do: helm install -f myvalues.yaml my-app ./app Would you be able to get a running app in your cluster? If so, would be possible to get from you a minimum reproducible example to work with? Lastly, we deliberately don't support YAML anchors and the likes mostly because we would have to do two passes: the first one to convert anchors to real values, so then in the second pass, files could be split... Otherwise, you risk splitting a file and an anchor all on different files. |
Another note: in this case:
You got a {{.kind | lower}}-{{.metadata.name}}.yaml And this is mentioned in the homepage. Since one of your resources doesn't have a name, you could default it: helm template my-release . \
-f values.yaml \
-f prod.yaml \
-n production | kubectl slice \
-o output-production-1 \
--template '{{.kind | lower}}-{{.metadata.name | default "unnamed" }}.yaml' # note this line This will correctly process the list of files above, rendering something like:
Where |
TL;DR: I guess my concern isn't about Lists, but about how nameless or non-k8s resources are handled when using include/exclude flags. A list for me is a construct for using YAML anchors and aliases. That's it. Packaging kube resources. I don't care that Kubernetes doesn't know about them. I use lists to avoid repetition on YAML. I have no need to do Example of DRY across resources and within a resource: ---
apiVersion: v1
kind: List # We're using a List so that we can use YAML anchors/aliases across multiple kube resources. Since they're in a single kubeyaml document/list, this works.
metadata:
name: name-is-sort-of-not-part-of-the-spec
items:
- kind: Service
apiVersion: v1
metadata:
name: &common-name my-repeated-resource-name
spec:
type: NodePort
selector:
app: *common-name
ports:
- name: http
protocol: TCP
port: &common-port 80
targetPort: application
- kind: Ingress
apiVersion: networking.k8s.io/v1
metadata:
name: *common-name
annotations:
kubernetes.io/ingress.class: "ingress-nginx"
nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
nginx.ingress.kubernetes.io/proxy-body-size: "100m"
spec:
tls:
- secretName: *common-name
hosts:
- &common-hostname my-repeated-hostname.example.com
rules:
- host: *common-hostname
http:
paths:
- backend:
service:
name: *common-name
port:
number: *common-port
pathType: ImplementationSpecific
That's fine. I suppose. I'll just start using names for lists when I need to use kubectl explain list
error: couldn't find resource for "/v1, Kind=List"
I appreciate your asking for clarification. We're probably getting lost in the semantics together. If Maybe the My workaround when I want to use either of those flags, is to set a name on the list. Maybe I should be using a name as a general practice. 🤷 It's not a hill I want to die on. 🤣 I was just used to not using a name up until the time that I wanted to exclude certain kinds.
I'll just start naming my lists. I guess I'm being stubborn. I did want to note the discrepancy in the techniques above.
I am just using Helm:
Kustomize:
I'm not interested in One would be correct to say that using anchors in helm is an anti-pattern. True, the go-templates are more flexible. But anchors work great with kustomize. Thanks for taking the time to collaborate. I'll name my lists. 😄 |
This is very helpful and a clever implementation. Thanks! |
^ The command with flag helm template blah . | kubectl slice --exclude-kind Pod -o output-dir --template '{{.kind | lower}}-{{.metadata.name | default "unnamed" }}.yaml'
error: unable to find Kubernetes "metadata.name" field in file number 9 |
I see what you mean. The implementation for
Since we don't know what you might want, and All of the I'll update the title of this issue to reflect that maybe what we want to do is not to enforce names if you're doing Then the issue might be that someone might come up with the opposite of your proposal: that since this is a Kubernetes plugin, there's an expectation that a name must be provided, because that's an "appropriate" Kubernetes manifest. We can cross that bridge when we get there 😆 I'll keep the issue open so other people might chime in. If there's enough support for this feature, I'll be more than happy to implement it, but as of now, it's a bit of a harder sale with the details considered above (rewriting the core so the |
I might have a working solution with a workaround: adding 2 new flags: one to allow empty names, and another to allow empty kinds. If you allow empty names, yet you decide to filter by name, it'll error our. If you allow empty names but you decide to filter by kind, then that's what you originally wanted so it will allow the request and process it. Give me a few days and I'll put something up. |
Hey @matthewhembree! The workaround has been released as part of 1.2.7. Try it out and let me know! I've been using it for a few weeks now to good success (ran into a similar issue than yours). For the time being, I'm closing this issue, but feel free to reopen and/or post comments with your thoughts! |
What did you do?
What did you expect to see?
All kinds except what was requested to be excluded. Including a
List
output aslist-.yaml
, which is normally output when no--include*
or--exclude*
flags are present.Expected:
What did you see instead?
^ File number 8 is the list.
If I'm just doing
--exclude-kind
,slice
shouldn't care aboutmetadata.name
Addingmetadata.name
does not make a difference.I'm not personally looking to have lists split recursively like #28.
I really want
slice
to be more passive with lists. It expects to seemetadata.name
which runs fine when you don't use any--include*
orexclude*
flags.Thanks!
The text was updated successfully, but these errors were encountered: