Skip to content
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

Allow setting every array element in replacements #4053

Closed
yogeshgadge opened this issue Jul 9, 2021 · 27 comments · Fixed by kubernetes-sigs/cli-experimental#240
Closed
Assignees
Labels
area/api issues for api module help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@yogeshgadge
Copy link

On the lines of targeting specific array entry prop.[name=nginx].host I tried entry prop.-.host so that all the array elements are targeted. However this did not work.

See my example below
kustomization.yaml

resources:
- configmap.yaml
- virtual-services.yaml
replacements:
- source:
     kind: ConfigMap
     fieldPath: data.MY_HOST
  target:
    kind: VirtualService
    fieldPaths:
     -  spec.http.-.rpute.0.destination.host

Here are the detailed values https://stackoverflow.com/questions/68322864/kustomize-how-to-target-replacement-in-every-array

I am using kustomize 4.2.0 version

@k8s-ci-robot k8s-ci-robot added needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 9, 2021
@natasha41575 natasha41575 added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Jul 9, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jul 9, 2021
@natasha41575 natasha41575 added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 9, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-kind Indicates a PR lacks a `kind/foo` label and requires one. label Jul 9, 2021
@natasha41575 natasha41575 added the area/api issues for api module label Jul 9, 2021
@natasha41575
Copy link
Contributor

natasha41575 commented Jul 9, 2021

There is no mechanism to do so, but we can consider adding one.

@monopole @KnVerey any concerns or objections with a mechanism to target all array elements in a replacement fieldPath?

@natasha41575
Copy link
Contributor

In its current state you would have to list out all the fieldPaths you need.

@natasha41575 natasha41575 added triage/under-consideration and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Jul 9, 2021
@k8s-ci-robot
Copy link
Contributor

@yogeshgadge: 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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jul 9, 2021
@natasha41575 natasha41575 removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jul 9, 2021
@yogeshgadge
Copy link
Author

yogeshgadge commented Jul 12, 2021

Using [] or .. for representing all elements seems reasonable to me.
IMO * makes more sense than empty or [
]

spec.http.*.route.0.destination.host

@KnVerey
Copy link
Contributor

KnVerey commented Jul 15, 2021

I think the feature makes sense. As for the syntax, fieldPath is essentially using a subset of JSONPath syntax, so we should stick with its way of expressing things as we add features. Specifically, our implementation should be consistent with what k/k packages use, i.e. this parser from client-go if my searching is serving me well. In this case, list[*].

@natasha41575 natasha41575 self-assigned this Jul 20, 2021
@natasha41575 natasha41575 added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed triage/under-consideration labels Jul 20, 2021
@natasha41575
Copy link
Contributor

natasha41575 commented Jul 20, 2021

Being consistent with k/k sounds good to me. In that case, the syntax would be something like spec.http.[*].route.0.destination.host when implemented.

@natasha41575
Copy link
Contributor

natasha41575 commented Sep 13, 2021

Adding a comment to note a caveat in that fieldPath slightly differs from JSONPath syntax.

fieldPath separates on "." for everything - so while a JSON path is something like spec.http[1].route, the equivalent kustomize replacements path would be spec.http.1.route. In order to be analogous to the current fieldPath syntax, the syntax for setting every array element should actually be spec.http.*.route.

@m-Bilal, feel free to assign yourself if you would like

@natasha41575 natasha41575 removed their assignment Sep 14, 2021
@m-Bilal
Copy link
Member

m-Bilal commented Sep 14, 2021

/assign

@k8s-triage-robot
Copy link

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 13, 2021
@siegenthalerroger
Copy link

Has there been any work on this topic?

@KnVerey
Copy link
Contributor

KnVerey commented Dec 15, 2021

@m-Bilal are you still planning to work on this?

@m-Bilal
Copy link
Member

m-Bilal commented Dec 16, 2021

Sorry I have not been able to work on this yet, been busy with office work. I do plan on working on this, but I'll be occupied this week as well. If anyone else wants to take this up, please feel free to do so. In case nobody takes this up, I'll leave a comment here when I do start working on this.

@k8s-triage-robot
Copy link

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

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

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

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 15, 2022
@siegenthalerroger
Copy link

A maintainer should probably remove the lifecycle/rotten and stale from this, seeing as it's still active and relevant

@m-Bilal
Copy link
Member

m-Bilal commented Jan 17, 2022

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jan 17, 2022
@natasha41575
Copy link
Contributor

/help

@k8s-ci-robot
Copy link
Contributor

@natasha41575:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

  • Why are we solving this issue?
  • To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
  • Does this issue have zero to low barrier of entry?
  • How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

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.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jan 21, 2022
@natasha41575 natasha41575 changed the title [Question] How to set property every array element ? Allow setting every array element in replacements Jan 21, 2022
@koba1t
Copy link
Member

koba1t commented Jan 26, 2022

/assign

@natasha41575
Copy link
Contributor

This is fixed by #4424. The only thing left to do is documentation.

@koba1t would you be interested in updating the documentation for this feature? The replacement doc lives here: https://github.com/kubernetes-sigs/cli-experimental/tree/master/site/content/en/references/kustomize/kustomization/replacements

@koba1t
Copy link
Member

koba1t commented Feb 13, 2022

I'm creating PR to add a document. kubernetes-sigs/cli-experimental#240
Do you think it needed to add an example?

@natasha41575
Copy link
Contributor

Documentation LGTM. Will merge the docs PR when we do a release. Thank you!

@tis-rpage
Copy link

I think there's a problem with this code.
kustomize version

{Version:kustomize/v4.5.3 GitCommit:de6b9784912a5c1df309e6ae9152b962be4eba47 BuildDate:2022-03-24T20:51:20Z GoOs:linux GoArch:amd64}

kustomize build test/

Error: wrong Node Kind for spec.template.spec.containers expected: MappingNode was SequenceNode: value: {- name: controller
  image: public.ecr.aws/aws-controllers-k8s/rds-controller:v0.0.19
  imagePullPolicy: IfNotPresent
  env:
  - name: AWS_REGION
  - name: AWS_ENDPOINT_URL
    value: ""
  command:
  - ./bin/controller
  args:
  - --aws-region
  - $(AWS_REGION)
  - --aws-endpoint-url
  - $(AWS_ENDPOINT_URL)
  ports:
  - containerPort: 8080
    name: http
  resources:
    limits:
      cpu: 100m
      memory: 128Mi
    requests:
      cpu: 50m
      memory: 64Mi
  securityContext:
    allowPrivilegeEscalation: false
    capabilities:
      drop:
      - ALL
    privileged: false
    runAsNonRoot: true}

File: test/deployment.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  namespace: test
  name: test
spec:
  replicas: 1
  template:
    spec:
      containers:
        - name: controller
          image: public.ecr.aws/aws-controllers-k8s/rds-controller:v0.0.19
          imagePullPolicy: IfNotPresent
          env:
            - name: AWS_REGION
            - name: AWS_ENDPOINT_URL
              value: ""
          command:
            - ./bin/controller
          args:
            - --aws-region
            - $(AWS_REGION)
            - --aws-endpoint-url
            - $(AWS_ENDPOINT_URL)
          ports:
            - containerPort: 8080
              name: http
          resources:
            limits:
              cpu: 100m
              memory: 128Mi
            requests:
              cpu: 50m
              memory: 64Mi
          securityContext:
            allowPrivilegeEscalation: false
            capabilities:
              drop:
                - ALL
            privileged: false
            runAsNonRoot: true
      hostIPC: false
      hostNetwork: false
      hostPID: false
      nodeSelector:
        kubernetes.io/os: linux
      terminationGracePeriodSeconds: 10

File: test/kustomization.yaml

resources:
  - deployment.yaml

configMapGenerator:
  - name: aws
    options:
      disableNameSuffixHash: true
    literals:
      - AWS_REGION=us-east-1

replacements:
  - source:
      kind: ConfigMap
      name: aws
      fieldPath: data.AWS_REGION
    targets:
      - options: { create: true }
        select:
          kind: Deployment
        fieldPaths:
          - spec.template.spec.containers.*.env.[name=AWS_REGION].value
# this works if uncommented
#  - source:
#      kind: ConfigMap
#      name: aws
#      fieldPath: data.AWS_REGION
#    targets:
#      - options: { create: true }
#        select:
#          kind: Deployment
#        fieldPaths:
#          - spec.template.spec.containers.[name=controller].env.[name=AWS_REGION].value

@natasha41575
Copy link
Contributor

@koba1t could you please investigate the above issue?

@koba1t
Copy link
Member

koba1t commented Mar 29, 2022

I was thinking of no use case to create nodes when wildcard matching (and PathMatcher didn't support create option).
So, I disable wild card matching when the Create option is enabled, and show the error message if using a wild card and the Create option was enabled.

I feel this bug is caused to output error messages. I can fix it.
So, do you think you need create option when using wild card?

@tis-rpage
Copy link

I would very much expect a wildcard and create options to be a supported combination. If you need to add an environment variable or an argument to every pod/deployment/statefulset, then that feature would be required. Without that option, we're only able to modify environment/arguments that are already defined with a new value.

@natasha41575
Copy link
Contributor

I see there is a new issue filed regarding this, so let's move the discussion there.

#4561

@natasha41575
Copy link
Contributor

I left a commend #4561 (comment)

I think we should start by improving the error message, but in general I agree that the feature makes sense to support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api issues for api module help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants