search

kubernetes-sigs / kustomize

Customization of kubernetes YAML configurations
Apache License 2.0
11.05k stars 2.25k forks source link

Prefix/Suffix transformers cannot target list elements #4884

Open nbjohnson opened 1 year ago

nbjohnson commented 1 year ago

I am trying to use a Prefix/Suffix transformer on my Ingress TLS hosts hostname, but it cannot seem to target that field. Below is my sample transformer and ingress configs. I have tried many variations on spec/tls/hosts but I cannot seem to target that list hostname object. The key here is I want to be able to prefix any name, hostname, or secretNames that are listed and I can get the others just not this last one. Any help would be appreciated as I know other people have had issues with list objects previously, not sure if I am missing something or if this is not currently possible.

apiVersion: builtin
kind: prefixTransformer
metadata:
  name: prefixer
prefix: "prefix-"
fieldSpecs:
- path: metadata/name
- path: spec/rules/host
  kind: Ingress
- path: spec/tls/secretName
  kind: Ingress
- path: spec/tls/hosts
  kind: Ingress
kind: Ingress
metadata:
  name: test-ingress
  namespace: test
spec:
  rules:
  - host: test.fakedomain.com
    http:
    ...
  tls:
  - hosts
    - test.fakedomain.com    <--- trying to target
    secretName: test-secret
k8s-ci-robot commented 1 year ago

@nbjohnson: 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 triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
KnVerey commented 1 year ago

I believe you are correct that it isn't possible to target that field with the prefix or suffix transformers. Those transformers can traverse through lists, but they require the terminal node to be a scalar. The code for the traversal is here, and the code that expects the field we land on to be a scalar is here.

/retitle Prefix/Suffix transformers cannot target list elements /triage under-consideration /kind feature

As an admittedly verbose workaround, the replacements transformer is capable of targeting that field. You'll just need to make sure the prefix transformer runs first, which you can do either but using both through the transformers field, or by putting your custom fieldSpecs into `configurations. For example:

# kustomization.yaml
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
  - resources.yaml

namePrefix: prefix-

configurations:
  - my-config.yaml

replacements:
  - path: replacement.yaml
# my-config.yaml
namePrefix:
- path: spec/rules/host
  kind: Ingress
- path: spec/tls/secretName
  kind: Ingress
nameSuffix:
- path: spec/rules/host
  kind: Ingress
- path: spec/tls/secretName
  kind: Ingress
# replacement.yaml
source:
  kind: Ingress
  name: test-ingress
  fieldPath: .spec.rules.0.host
targets:
- select:
    name: test-ingress
    kind: Ingress
  fieldPaths:
  - spec.tls.0.hosts.0

Result:

kind: Ingress
metadata:
  foo: bar
  name: prefix-test-ingress
  namespace: test
spec:
  rules:
  - host: prefix-test.fakedomain.com
  tls:
  - hosts:
    - prefix-test.fakedomain.com
    secretName: prefix-test-secret
IvanovOleg commented 1 year ago

@KnVerey I have a similar problem, but a bit more complex. I need a dynamic prefix for my istio VirtualService in order to use it for PR environments. foo- prefix wasn't substituted. Also I see that argocd-vault-plugin wasn't able to substitute value, but it is not important at the moment.

kustomization.yaml:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
- ../../base

configurations:
  - namereference.yaml

transformers:
  - replacement1.yaml
  - prefixsuffix.yaml
  - replacement2.yaml

namereference.yaml:

nameReference:
  - kind: Service
    fieldSpecs:
      - kind: Namespace
        path: metadata/name

replacement1.yaml:

apiVersion: builtin
kind: ReplacementTransformer
metadata:
  name: prefix
replacements:
  - source:
      kind: VirtualService
      fieldPath: metadata.name
    targets:
      - select:
          kind: PrefixSuffixTransformer
        fieldPaths:
          - prefix

prefixsuffix.yaml

apiVersion: builtin
kind: PrefixSuffixTransformer
metadata:
  name: host
prefix: "foo-"
fieldSpecs:
  - kind: VirtualService
    path: metadata/annotations/host

replacement2.yaml:

apiVersion: builtin
kind: ReplacementTransformer
metadata:
  name: host
replacements:
  - source:
      kind: VirtualService
      fieldPath: metadata.annotations.host
    targets:
      - select:
          kind: VirtualService
        fieldPaths:
          - spec.hosts.0

virtualservice.yaml (in base):

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: example
  annotations:
    host: <path:secret#domain>
spec:
  hosts:
    - "<path:secret#domain>"
  gateways:
    - istio-gateway/istio-gateway
  http:
    - route:
      - destination:
          host: example
          port:
            number: 8000

Actual result:

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  annotations:
    host: 'foo-<path:secret#domain>'
  labels:
    app.kubernetes.io/instance: example-pr-10
  name: example-pr-10
  namespace: example-pr-10
spec:
  gateways:
    - istio-gateway/istio-gateway
  hosts:
    - 'foo-<path:secret#domain>'
  http:
    - route:
        - destination:
            host: example
            port:
              number: 8000

Expected result:

apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  annotations:
    host: 'example-pr-10.example.com'
  labels:
    app.kubernetes.io/instance: example-pr-10
  name: example-pr-10
  namespace: example-pr-10
spec:
  gateways:
    - istio-gateway/istio-gateway
  hosts:
    - 'example-pr-10.example.com'
  http:
    - route:
        - destination:
            host: example
            port:
              number: 8000
nbjohnson commented 1 year ago

@KnVerey Thanks for the suggestion, I was actually trying to use the replacement transformer similar to your suggestion as a workaround, but I can't quite get it to work with my situation. I want it to be able to be general so in this case if there are multiple ingresses it will update all of them. However, I keep getting the error Error: multiple matches for selector Ingress.[noVer].[noGrp]/[noName].[noNs]:.spec.rules.0.host as I leave out the name compared to your example. I assume this is just a limitation of the replacement transformer, but is it possible to make the replacement transformer relative so it can work generally across multiple ingresses?

apiVersion: builtin
kind: ReplacementTransformer
metadata:
  name: configReplacement
replacements:
- source:
    kind: Ingress
    fieldPath: .spec.rules.0.host
  targets:
  - select:
      kind: Ingress
    fieldPaths:
    - spec.tls.0.hosts.0
KnVerey commented 1 year ago

@IvanovOleg at a glance I see two possible issues with the provided configuration. One is that transformers in the transformers field require fieldspecs to be provided as part of the transformer yaml; anything provided through configurations is overridden when using that method. Second, you seem to be attempting to modify the transformer config using replacements. That is possible, but it's an advanced pattern called "transformed tranformers" that requires a bit of a different setup, which you can read about here: https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/#transformed-transformers.

@nbjohnson that error is coming from having multiple ingresses targeted by the source field, and that is indeed a limitation of how the transformer works. It starts by finding the one specific value to use as the replacement, then identifies all the target spots in one or more target objects and inserts that value. There's no connection between the source and the target. It sounds like what you want is a self-reference feature, which is not currently available but does sound useful. I think there's enough to think about that it should be proposed in a mini enhancement proposal if you're interested.

nbjohnson commented 1 year ago

@KnVerey Are you suggesting opening an enhancement proposal for a change to prefix/suffix transformer in order to be able to target a terminal node that in non-scalar? Or for a proposal on adding a self-reference feature for the replacements transformer? Or for a discussion on what might be overall the most useful between the two, while also solving my issue? Sorry just want to clarify what proposal you are suggesting I open

KnVerey commented 1 year ago

Are you suggesting opening an enhancement proposal for a change to prefix/suffix transformer in order to be able to target a terminal node that in non-scalar? Or for a proposal on adding a self-reference feature for the replacements transformer?

I meant for the self-reference feature for the replacements transformer. The ability to target non-scalar terminal nodes has precedent in other transformers in terms of what the fieldspecs would look like, so there's not much to design there--this issue itself is sufficient for tracking the feature request.

Or for a discussion on what might be overall the most useful between the two, while also solving my issue?

Thinking more about your goal, what bothers me about both solutions is that they both update the target fields pretty blindly. Say your hosts list contained some entries that shouldn't be prefixed. Even if we implemented both options discussed so far, they would only be capable of updating all the entries, or at best specific entries by individually specified indexes (not ideal, since order != identity). The self-reference approach points out something important: we are dealing with several fields that all actually refer to the same name and therefore need to be updated together. Kustomize is able to do that for object names referenced in arbitrary places, including scalar lists. So, another approach would be to use a fake, local-only Kind to tie all these name references together, including any you may have in objects other than Ingress. This almost works, but only if the fake objects are not stripped--I think this is a bug, and one that is trivial (unlike the other changes we've been talking about so far) to fix. I threw up a quick PR with the fix, and it includes a test demonstrating this technique. PTAL--I'm curious to hear what you think of this approach, and whether it would work for you in practice: https://github.com/kubernetes-sigs/kustomize/pull/4895

https://github.com/kubernetes-sigs/kustomize/blob/fb0f567b632bbf2dc8c6e0e2a4ac1d4ca717f3f5/api/krusty/namereference_test.go#L675-L787

nbjohnson commented 1 year ago

@KnVerey That local reference method is interesting, I don't know too much about it and can't seem to find docs on it. Seems quite useful being able to directly reference a pairing like that and have it be able to differentiate between different hosts. However, at least for my need I am not sure that will work as that requires knowledge of the hostnames. In my situation I do not have advance knowledge of the hostnames, I just need to prefix all that I receive, so unless there is a way for me to extract all of the hostnames and generate those fake objects on the fly not sure it helps my situation but might be useful for someone else's scenario. I understand the hesitation with only being able to update all the entries, but at least for my need right now that is exactly what I need to do. I just want to be able to target all Ingresses and update all of their name references, so metadata name, tls secret name, and the hostnames. I understand that is probably undesirable in most scenarios for people, but the prefix/suffix transformer is able to target names indiscriminately, but just not the tls hostname in Ingress.

nbjohnson commented 1 year ago

@KnVerey Just want to follow-up, any additional thoughts on this? I do need to be able to mass prefix all parts of my ingress hostnames

nbjohnson commented 1 year ago

@KnVerey Wanted to follow-up as I am still in need of being able to target Ingress spec.tls.*.hosts.* to be able to prefix. Your above name reference example does look interesting, but I don't think it would work in my case, that looks like I need to define IngressHost ahead of time, but in my situation I am generating these on the fly, so I do not know the hostname ahead of time to define that, unless there is a way to fetch all of the Ingress hostnames and generate those on the fly.

endersonmaia commented 1 year ago

I have a situation where I want to add a suffix to containers/0/env/0/secretKeyRef/name

It works for metadata/name but not for the env.

---
apiVersion: builtin
kind: PrefixSuffixTransformer
metadata:
  name: networkSuffix
suffix: "-goerli"
fieldSpecs:
  - kind: Deployment
    path: metadata/name
  - kind: Deployment
    path: spec/template/spec/containers/0/env/0/valueFrom/secretKeyRef/name

I get:

Error: accumulating components: accumulateDirectory: "recursed accumulation of path 'postgraphile/components/goerli': considering field 'spec/template/spec/containers/0/env/0/valueFrom/secretKeyRef/name' of object Deployment.v1.apps/postgraphile-goerli.namespace: visit traversal on path: [0 env 0 valueFrom secretKeyRef name]: fieldName: 0: wrong node kind: expected SequenceNode but got MappingNode
...
k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 1 year ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

init-penguin commented 1 year ago

~I'm having the same problem with spec.tls.hosts.* with replacements, while spec.rules.*.host compiles as expected.~ My bad – messed up pathspec for replacement. It works 😅

/remove-lifecycle rotten

Zegorax commented 1 year ago

Have you found a workaround for this feature ? I'm trying to achieve the same operation as you. (Adding a suffix to the TLS host of an Ingress dynamically, as it may contain multiple ones)

init-penguin commented 1 year ago

@Zegorax hey! I personally ended up switching to grafana tanka/jsonnet entirely, with no issues so far. Requires some brain reconfiguration, yes, but totally worth it.

eternalphane commented 10 months ago

@endersonmaia You should use [0] instead of /0, see #4392

Edit: It doesn't work

ilyavaiser commented 9 months ago

Kustimize is a serious tool, but the simplest task - how to add a suffix to a dynamically generated service name and add this value to the first element of the array in the hosts field in another kind - seems impossible.

This ticket is the latest point in my journey of trying to resolve this issue. I’ll subscribe, what if someday this will become possible?

k8s-triage-robot commented 6 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 5 months ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

DanHam commented 4 months ago

/remove-lifecycle rotten

DanHam commented 4 months ago

@endersonmaia This is probably way too late. However, it may help some others who read your post and are possibly experiencing the same issue as you did.

The following should work:

---
apiVersion: builtin
kind: PrefixSuffixTransformer
metadata:
  name: networkSuffix
suffix: "-goerli"
fieldSpecs:
  - kind: Deployment
    path: metadata/name
  - kind: Deployment
    path: spec/template/spec/containers[]/env[]/valueFrom/secretKeyRef/name

Note that there is no path separator / between e.g. containers and the pair of square braces.

You cannot use an index inside the braces to target a specific element of the array / list e.g. [0]. Nor can you target an element of an array / list by using some match criteria inside the braces e.g. [name=foo].

Strange that this doesn't work quite the same way as other transformers....

lucasalvarezlacasa commented 2 months ago

Any workarounds for this? I'm struggling with the same problem with no apparent solution.

Zegorax commented 2 weeks ago

Ciao everyone,

@KnVerey is there any update about this one ? I'm still stuck with some automated ArgoCD workloads due to this. Thanks a lot in advance!