patch-operator

patch-operator copied to clipboard

I faced an issue with a patch which appends items to an array.

[
    {
        "op": "add",
        "path": "/spec/install/spec/deployments/0/spec/template/spec/containers/0/args/-",
        "value": "--dns01-recursive-nameservers-only"
    },
    {
        "op": "add",
        "path": "/spec/install/spec/deployments/0/spec/template/spec/containers/0/args/-",
        "value": "--dns01-recursive-nameservers=8.8.8.8:53,1.1.1.1:53"
    }
]

This patch is not idempotent and the issue is that patch operator tries to apply continuously until your etcd will eat up all the available memory. I think this behaviour is expected with this kind of patches, but I still think it would be good to have a safety mechanism to avoid this since it can have really bad consequences.

this is a common issue when dealing with arrays. Please see this #28 also as it was around the same problem.

from what I understand from the linked issue, even if the above patch is indeed also about cert-manager, it is not linked to the cert-manager deployed by the patch controller (as I don't use that). I think this problem is more generic, as indeed you can face it with any patch dealing with array.

right and the solution I proposed was to use a logic by which you first check whether the desired element exists and: if it exists, you return the current array if it does not exist you return the current array with the new element.

On Mon, Jun 27, 2022 at 10:33 AM Andrea Decorte @.***> wrote:

from what I understand from the linked issue, even if the above patch is indeed also about cert-manager, it is not linked to the cert-manager deployed by the patch controller (as I don't use that). I think this problem is more generic, as indeed you can face it with any patch dealing with array.

— Reply to this email directly, view it on GitHub https://github.com/redhat-cop/patch-operator/issues/29#issuecomment-1167049955, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPERXB7AUD45SOHBUUPSFDVRFRNJANCNFSM5ZKFZ6FQ . You are receiving this because you commented.Message ID: @.***>

-- ciao/bye Raffaele

thanks, just to be aligned, you mean this kind of workaround right? That should work too, or my current workaround is to pass the whole array to patch.

Then I still think it would be interesting to have a safety mechanism to avoid infinite patching (a dummy one could be an overridable size limit on the target object).

I do not understand why the the following snippet could do an infinite patch:

      patchTemplate: '[{"op": "add", "path": "/spec/identityProviders/-","value":{"name": "keycloak","type": "OpenID","mappingMethod": "claim","openID": {"claims": {"email":["email"],"name":["name"],"preferredUsername":["preferred_username"]},"clientID": "ocp-login","clientSecret": {"name": "keycloak-client-secret"},"issuer": "https://xxx"}} }]'
      patchType: application/json-patch+json

Do I miss anything here because i do not add 2 elements for the same array position?

Best Alex

path": "/spec/identityProviders/-"

because this one appends to the end of the array, it doesn't check if the item is already there, so running your patch twice will append the value twice

path": "/spec/identityProviders/-"

because this one appends to the end of the array, it doesn't check if the item is already there, so running your patch twice will append the value twice

How can one fix this behaviour then? I'm having the same issue. Is there some way around that? I tried with various add paths e.g. /.../.../1 or /.../.../2 but it's still generating the entries infinite.

I'll share my findings with you :) Finally got something working to patch my SCCs and keep already existing configurations!!!

---
apiVersion: redhatcop.redhat.io/v1alpha1
kind: Patch
metadata:
  name: whatever-patch
  namespace: whatever-namespace
spec:
  serviceAccountRef:
    name: whatever-user
  patches:
    whatever-patch:
      targetObjectRef:
        apiVersion: security.openshift.io/v1
        kind: SecurityContextConstraints
        name: privileged
      patchTemplate: |
        users:
        {{- if (not (has "system:serviceaccount:whatever-namespace:whatever" (index . 0).users)) }}
        {{ append (index . 0).users "system:serviceaccount:whatever-namespace:whatever" | toYaml }}
        {{- else }}
        {{ (index . 0).users | toYaml }}
        {{- end }}
      patchType: application/merge-patch+json

@Ninsbean on a site-node, why don't you use RBAC for SCC?

Does anybody have a running example for kind: OAuth? I'm not able to get it running.

The following patch works but I can't inject the ldapIDP var:

apiVersion: redhatcop.redhat.io/v1alpha1
kind: Patch
metadata:
  name: oauth-patch
  namespace: ccb-openshift-config
spec:
  serviceAccountRef:
    name: patch-operator
  patches:
    oauth:
      targetObjectRef:
        apiVersion: config.openshift.io/v1
        kind: OAuth
        name: cluster
      patchTemplate: |
        {{ $ldapIDP:= dict "name" "ldap-test" "type" "LDAP" "ldap" (dict "url" "ldap://test.ch" "attributes" (dict "id"  (list "dn"))) }}
        spec:
          identityProviders:
          - htpasswd:
              fileData:
                name: htpass-secret
            mappingMethod: claim
            name: htpasswd-provider
            type: HTPasswd
      patchType: application/merge-patch+json

The following patch doesn't work:

apiVersion: redhatcop.redhat.io/v1alpha1
kind: Patch
metadata:
  name: oauth-patch
  namespace: ccb-openshift-config
spec:
  serviceAccountRef:
    name: patch-operator
  patches:
    oauth:
      targetObjectRef:
        apiVersion: config.openshift.io/v1
        kind: OAuth
        name: cluster
      patchTemplate: |
        {{ $ldapIDP:= dict "name" "ldap-test" "type" "LDAP" "ldap" (dict "url" "ldap://test.ch")(dict "attributes" (dict "id"  (list "dn"))) }}
        spec:
          identityProviders:
          {{- if (not (has $ldapIDP (index . 0).spec.identityProviders)) }}
          {{ append (index . 0).spec.identityProviders $ldapIDP | toYaml }}
          {{- else }}
          {{ (index . 0).spec.identityProviders | toYaml }}
          {{- end }}
      patchType: application/merge-patch+json

error-message: :1: unexpected "(" in operand

here is one I use: https://github.com/raffaelespazzoli/backstage-demo/blob/668c757d54b1949f1af93d3f6bf9c7edb06b5ca0/openshift-config/openshift-config/templates/patches.yaml#L154

Thank you very much for your patch-specs. Unfortunately I'm still not able to get it running. OCP 4.9.43.

apiVersion: redhatcop.redhat.io/v1alpha1
kind: Patch
metadata:
  name: oauth-patch
  namespace: ccb-openshift-config
spec:
  serviceAccountRef:
    name: patch-operator
  patches:
    oauth:
      targetObjectRef:
        apiVersion: config.openshift.io/v1
        kind: OAuth
        name: cluster
      patchTemplate: |
        {{ "{{-" }} $backStageDemoIDP:= dict "name" "backstage-demo-github" "mappingMethod" "claim" "type" "GitHub" "github" (dict "teams" (list) "clientID" "clientIDsdfa" "clientSecret" (dict "name" "ocp-github-app-credentials") "organizations" (list ("org")) ) {{ "-}}" }}
        spec:
          identityProviders:
          {{ "{{-" }} if (not (has $backStageDemoIDP (index . 0).spec.identityProviders)) {{ "}}" }}
        {{ "{{" }} append (index . 0).spec.identityProviders $backStageDemoIDP | toYaml | indent 4 {{ "}}" }}
          {{ "{{-" }} else {{ "}}" }}
        {{ "{{" }} (index . 0).spec.identityProviders | toYaml | indent 4 {{ "}}" }}
          {{ "{{-" }} end {{ "}}" }}
      patchType: application/merge-patch+json

error-message: Message: yaml: did not find expected node content

{"level":"error","ts":1661347304.2516704,"logger":"patch-reconciler.ccb-openshift-config/oauth-patch.oauth","msg":"unable to convert to json","processed template":"{{- $backStageDemoIDP:= dict \"name\" \"backstage-demo-github\" \"mappingMethod\" \"claim\" \"type\" \"GitHub\" \"github\" (dict \"teams\" (list) \"clientID\" \"clientIDsdfa\" \"clientSecret\" (dict \"name\" \"ocp-github-app-credentials\") \"organizations\" (list (\"org\")) ) -}}\nspec:\n  identityProviders:\n  {{- if (not (has $backStageDemoIDP (index . 0).spec.identityProviders)) }}\n{{ append (index . 0).spec.identityProviders $backStageDemoIDP | toYaml | indent 4 }}\n  {{- else }}\n{{ (index . 0).spec.identityProviders | toYaml | indent 4 }}\n  {{- end }}            \n","error":"yaml: did not find expected node content","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:311\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:266\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227"}
{"level":"error","ts":1661347304.2517767,"logger":"controller.patch-reconciler_oauth","msg":"Reconciler error","name":"cluster","namespace":"","error":"yaml: did not find expected node content","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:227"}

gomplate show's me that everything is correct, replaced list with slice:

> gomplate -i '{{ dict "name" "backstage-demo-github" "mappingMethod" "claim" "type" "GitHub" "github" (dict "teams" (slice) "clientID" "dasf" "clientSecret" (dict "name" "ocp-github-app-credentials") "organizations" (slice ("org1")) ) | data.ToYAML }}'
github:
  clientID: dasf
  clientSecret:
    name: ocp-github-app-credentials
  organizations:
    - org1
  teams: []
mappingMethod: claim
name: backstage-demo-github
type: GitHub

I'm able to add that part with oc edit oauth cluster. I've also tried the origin specs and generated the secrets and populated them with any possible data ("client_id", "client_secret", "GITHUB_ORG"). But still the same error :(

Best

sorry my bad=, I took that manifest from a helm template, so there are double parenthesis for escaping. If you replace {{ "{{-" }} with {{ and {{ "}}" }} with }} the template should compile.