Not able to patch Oauth with patchType: application/strategic-merge-patch+json

[donflavour]

I'm pretty sure that the following snippet worked before, but unfortunately with 0.1.4 I don't get it running anymore
OCP 4.8.35 & Patch operator 1.4.0

apiVersion: redhatcop.redhat.io/v1alpha1
kind: Patch
metadata:
  name: ldap
  namespace: my-namespace
spec:
  serviceAccountRef:
    name: patch-operator
  patches:
    ldap-oauth-provider:
      targetObjectRef:
        apiVersion: config.openshift.io/v1
        kind: OAuth
        name: cluster
      patchTemplate: |
        spec:
          identityProviders:
          - ldap:
              attributes:
                email:
                  - mail
                id:
                  - dn
                name:
                  - cn
                preferredUsername:
                  - cn
              bindDN: CN=<cn>,OU=<ou>,OU=<ou>,OU=<ou>,OU=<ou>,DC=<dc>,DC=<dc>
              bindPassword:
                name: ldap-secret
              ca:
                name: ldap-ca-bundle
              insecure: false
              url: "{{ (index . 1).data.url }}"
            mappingMethod: claim
            name: ldap
            type: LDAP   
      patchType: application/strategic-merge-patch+json
      sourceObjectRefs:
      - apiVersion: v1
        kind: ConfigMap
        name: ldap-url
        namespace: my-namespace

I'm receiving the following error message:

  Patch Statuses:
    Ldap - Oauth - Provider:
      /cluster:
        Last Transition Time:  2022-05-18T15:34:42Z
        Message:               the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json, application/apply-patch+yaml
        Observed Generation:   15
        Reason:                LastReconcileCycleFailed
        Status:                True
        Type:                  ReconcileError

When I change the patchType to application/merge-patch+json, the patch is successfully reconciled but the existing oauth part (htpasswd) is overwritten.

Best
Alex

[donflavour]

Hi @raffaelespazzoli

Could you help me with that issue?
I'm able to successfully apply the following patch:

apiVersion: redhatcop.redhat.io/v1alpha1
kind: Patch
metadata:
  name: i0-label-additional
spec:
  serviceAccountRef:
    name: patch-operator
  patches:
    i0-label:
      targetObjectRef:
        apiVersion: v1
        kind: Node
        name: infra-0-dev.sharedres.ch
      patchTemplate: |
        metadata:
          labels:
            test: test1
      patchType: application/strategic-merge-patch+json

Nevertheless the same patch for another resource is not working:

apiVersion: redhatcop.redhat.io/v1alpha1
kind: Patch
metadata:
  name: node-selector-openshift-pipelines
spec:
  serviceAccountRef:
    name: patch-operator
  patches:
    tekton-config:
      targetObjectRef:
        apiVersion: operator.tekton.dev/v1alpha1
        kind: TektonConfig
        name: config
      patchTemplate: |
        metadata:
          labels:
            test: test1
      patchType: application/strategic-merge-patch+json

Patch Statuses:
    Tekton - Config:
      /config:
        Last Transition Time:  2022-05-27T10:00:57Z
        Message:               the body of the request was in an unknown format - accepted media types include: application/json-patch+json, application/merge-patch+json, application/apply-patch+yaml
        Observed Generation:   5
        Reason:                LastReconcileCycleFailed
        Status:                True
        Type:                  ReconcileError
      Reconciler:
        Last Transition Time:  2022-05-27T10:00:55Z
        Message:
        Reason:                ReconcilerManagerRestarting
        Status:                True
        Type:                  Initializing
Events:                        <none>

Many Thanks

[raffaelespazzoli]

can you try to change your patch type to application/merge-patch+json.

[donflavour]

The patch is reconciled successfully but existing parts of the oauth (p.e. htpasswd) is overwritten.
We would like to split the oauth configuration to different patches.

[raffaelespazzoli]

this is how I solve that problem:

    github-ocp-oauth-provider:
      targetObjectRef:
        apiVersion: config.openshift.io/v1
        kind: OAuth
        name: cluster
      sourceObjectRefs:
      - apiVersion: v1
        kind: Secret
        name: ocp-github-app-credentials
        namespace: openshift-config  
      - apiVersion: v1
        kind: Secret
        namespace: backstage
        name: github-credentials        
      patchTemplate: |
        {{ "{{-" }} $backStageDemoIDP:= dict "name" "backstage-demo-github" "mappingMethod" "claim" "type" "GitHub" "github" (dict "teams" (list) "clientID" ((index . 1).data.client_id | b64dec) "clientSecret" (dict "name" "ocp-github-app-credentials") "organizations" (list ((index . 2).data.GITHUB_ORG | b64dec)) ) {{ "-}}" }}
        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

the patch logic checks for the existence of the item before adding it. It does not overwrite the entire array.
Note: this is an issue around how the OAuth object is defined, i.e. it is not patch friendly.
Let me know if you can figure out how to make it work for you from this example.
You might have to fix the parenthesis, this fragment comes from a helm chart so I needed to be sure helm would not try to interpret the patch

[donflavour]

Thanks for your suggestion - unfortunately I'm not able to make it working. I've created the sourceObjects and applied the following patch:

apiVersion: redhatcop.redhat.io/v1alpha1
kind: Patch
metadata:
  name: ldap
spec:
  serviceAccountRef:
    name: patch-operator
  patches:
    github-ocp-oauth-provider:
      targetObjectRef:
        apiVersion: config.openshift.io/v1
        kind: OAuth
        name: cluster
      sourceObjectRefs:
      - apiVersion: v1
        kind: Secret
        name: ocp-github-app-credentials
        namespace: openshift-config  
      - apiVersion: v1
        kind: Secret
        namespace: backstage
        name: github-credentials        
      patchTemplate: |
        {{ "{{-" }} $backStageDemoIDP:= dict "name" "backstage-demo-github" "mappingMethod" "claim" "type" "GitHub" "github" (dict "teams" (list) "clientID" ((index . 1).data.client_id | b64dec) "clientSecret" (dict "name" "ocp-github-app-credentials") "organizations" (list ((index . 2).data.GITHUB_ORG | b64dec)) ) {{ "-}}" }}
        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

After applying the controller shows the following error message:

{"level":"error","ts":1655882453.2933376,"logger":"patch-reconciler.ccb-openshift-config/ldap.github-ocp-oauth-provider","msg":"unable to convert to json","processed template":"{{- $backStageDemoIDP:= dict \"name\" \"backstage-demo-github\" \"mappingMethod\" \"claim\" \"type\" \"GitHub\" \"github\" (dict \"teams\" (list) \"clientID\" ((index . 1).data.client_id | b64dec) \"clientSecret\" (dict \"name\" \"ocp-github-app-credentials\") \"organizations\" (list ((index . 2).data.GITHUB_ORG | b64dec)) ) -}}\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/controller-runtime@v0.10.0/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/controller-runtime@v0.10.0/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/controller-runtime@v0.10.0/pkg/internal/controller/controller.go:227"}

Do you know what is causing the failure? I would like to make it working to play around with some additional usecases.
p.e. we can check the existence of an enabled bool in the sourceObject and append the particular identity-provider if the enable bool is true.

In the meantime I used the application/json-patch+json with a one liner.

      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

[raffaelespazzoli]

my example was supposed to be adapted to you use case. I probably was not going to work right away.
anyway it looks like you found an even better solution.
May I close this issue?