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

Replacements Transformer: Replace All Instances does not work with create:True #4561

Closed
vsabella opened this issue Apr 1, 2022 · 5 comments · Fixed by #4886
Closed

Replacements Transformer: Replace All Instances does not work with create:True #4561

vsabella opened this issue Apr 1, 2022 · 5 comments · Fixed by #4886
Assignees
Labels
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. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@vsabella
Copy link

vsabella commented Apr 1, 2022

When attempting this replacement with wildcard * it fails with
kustomize v4.5.4

Error: accumulating components: 
accumulateDirectory: "recursed accumulation of path '/envoyfilter-transformer/istio-1.12': 

accumulating components: accumulateDirectory: \"recursed accumulation of path '/envoyfilter-transformer/base': 
  wrong Node Kind for spec.configPatches expected: MappingNode was SequenceNode: value: 
apiVersion: kustomize.config.k8s.io/v1alpha1
kind: Component
replacements:
  - source:
      kind: ConfigMap
      name: istio-version
      fieldPath: data.ISTIO_REGEX
    targets:
      - select:
          kind: EnvoyFilter
        fieldPaths:
          - spec.configPatches.*.match.proxy.proxyVersion
          #- spec.configPatches.0.match.proxy.proxyVersion

        options:
          create: true
---
# https://docs.fastly.com/signalsciences/install-guides/kubernetes/kubernetes-istio/
apiVersion: networking.istio.io/v1alpha3
kind: EnvoyFilter
metadata:
  name: axon-sigsci
spec:
  workloadSelector:
    labels:
      istio: ingressgateway
  configPatches:
    - applyTo: HTTP_FILTER
      match:
        context: GATEWAY
        listener:
          filterChain:
            filter:
              name: "envoy.filters.network.http_connection_manager"
              subFilter:
                name: "envoy.filters.http.router"
      patch:
        operation: INSERT_FIRST
        value:
          name: my.ext_authz
          typed_config:
            "@type": "type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthz"
            transport_api_version: V3
            failure_mode_allow: true
            with_request_body:
              max_request_bytes: 16384
              allow_partial_message: true
            grpc_service:
              envoy_grpc:
                cluster_name: my-cluster
    - applyTo: CLUSTER
      patch:
        operation: ADD
        value:
          name: my-cluster
          type: STRICT_DNS
          connect_timeout: 0.5s
          http2_protocol_options: {}
          load_assignment:
            cluster_name: my-cluster
            endpoints:
              - lb_endpoints:
                  - endpoint:
                      address:
                        socket_address:
                          address: my-address
                          port_value: 1234
@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 Apr 1, 2022
@natasha41575
Copy link
Contributor

It makes sense to me in a lot of cases for the wildcard match to work with create: true. The example provided in this issue is a valid use case.

Implementation is a separate discussion, there may be some nitty gritty details with the way PathMatcher is currently implemented that could make this very difficult to do, but from the user perspective I agree that this is a desirable feature.

@koba1t would you be able to improve the error message, to something like wildcard match in replacements cannot be used with create: true option? I think we should at least do that, and then we can revisit implementation of the support later.

/triage accepted
/kind feature

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. kind/feature Categorizes issue or PR as related to a new feature. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Apr 1, 2022
@natasha41575 natasha41575 added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Apr 1, 2022
@koba1t
Copy link
Member

koba1t commented Apr 11, 2022

hi @natasha41575.
I fix the error message on #4577.
Could you check it?

And I'm watching your PR(#4574).
May I help you with your work?

@natasha41575
Copy link
Contributor

hi @natasha41575.
I fix the error message on #4577.
Could you check it?

And I'm watching your PR(#4574).
May I help you with your work?

Thanks! Your PR covers what I was trying to fix with mine, so let's go with yours.

@natasha41575
Copy link
Contributor

The error message is now improved.

If in the future we want to support using create: true with the wildcard match, I am open to accepting that, so I will leave this issue open in case someone wants to contribute it.

/lifecycle frozen

@KnVerey
Copy link
Contributor

KnVerey commented Sep 12, 2022

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. 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.

5 participants