fix: add missing label on Authronio CR to have istio sidecar

[zdtsw]

Description

How Has This Been Tested?

Screenshot or short clip

Merge criteria

• You have read the contributors guide.
• Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
• Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has manually tested the changes and verified that the changes work

[bartoszmajsak]

This will not work, labels from Operator CR are not propagated to Deployment's spec.template where we need them (sounds like a good contribution though).

That's why we have

opendatahub-operator/controllers/dscinitialization/servicemesh_setup.go

Lines 216 to 238 in fe44f28

	// We do not have the control over deployment resource creation.
	// It is created by Authorino operator using Authorino CR and labels are not propagated from Authorino CR to spec.template
	// See https://issues.redhat.com/browse/RHOAIENG-5494
	//
	// To make it part of Service Mesh we have to patch it with injection
	// enabled instead, otherwise it will not have proxy pod injected.
	feature.Define("enable-proxy-injection-in-authorino-deployment").
	Manifests(
	manifest.Location(Templates.Location).
	Include(path.Join(Templates.AuthorinoDir, "deployment.injection.patch.tmpl.yaml")),
	).
	PreConditions(
	func(ctx context.Context, cli client.Client, f *feature.Feature) error {
	namespace, err := servicemesh.FeatureData.Authorization.Namespace.Extract(f)
	if err != nil {
	return fmt.Errorf("failed trying to resolve authorization provider namespace for feature '%s': %w", f.Name, err)
	}
	return feature.WaitForPodsToBeReady(namespace)(ctx, cli, f)
	},
	).
	WithData(servicemesh.FeatureData.ControlPlane.Define(&instance.Spec).AsAction()).
	WithData(servicemesh.FeatureData.Authorization.All(&instance.Spec)...),

[dhirajsb]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dhirajsb
Once this PR has been reviewed and has the lgtm label, please ask for approval from zdtsw. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dhirajsb]

@bartoszmajsak this is a prerequisite to allow Authorino to copy that label to it's deployment. I'm not sure whether it actually does that or not.

[bartoszmajsak]

@bartoszmajsak this is a prerequisite to allow Authorino to copy that label to it's deployment.

@dhirajsb Please read the comment in the code I linked above.

I'm not sure whether it actually does that or not.

Then how come you /lgtm? :)

[bartoszmajsak]

I looked at the code in Authorino again (Kuadrant/authorino-operator#91) and it should work by quickly looking at unit tests, but there seems to be a bug, as after applying:

❯ kubectl apply  -f -<<EOF
apiVersion: operator.authorino.kuadrant.io/v1beta1
kind: Authorino
metadata:
  name: authorino
  labels:
    sidecar.istio.io/inject: "true"
spec:
  image: quay.io/kuadrant/authorino:latest
  replicas: 1
  clusterWide: true
  listener:
    tls:
      enabled: false
  oidcServer:
    tls:
      enabled: false
EOF

authorino.operator.authorino.kuadrant.io/authorino created

my deployment looks like that:

apiVersion: apps/v1
kind: Deployment
metadata:
  annotations:
    deployment.kubernetes.io/revision: "1"
  creationTimestamp: "2025-01-09T17:46:27Z"
  generation: 1
  labels:
    sidecar.istio.io/inject: "true"
  name: authorino
  namespace: default
  ownerReferences:
  - apiVersion: operator.authorino.kuadrant.io/v1beta1
    blockOwnerDeletion: true
    controller: true
    kind: Authorino
    name: authorino
    uid: 26155b7b-3ef0-4fb6-98d8-741a0c180f6f
  resourceVersion: "584"
  uid: f64c0bca-a799-4d15-b3e9-1c5018671b35
spec:
  progressDeadlineSeconds: 600
  replicas: 1
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      authorino-resource: authorino
      control-plane: controller-manager
  strategy:
    rollingUpdate:
      maxSurge: 25%
      maxUnavailable: 25%
    type: RollingUpdate
  template:
    metadata:
      creationTimestamp: null
      labels:
        authorino-resource: authorino
        control-plane: controller-manager
    spec:
      containers:
      - image: quay.io/kuadrant/authorino:latest
        imagePullPolicy: Always
        name: authorino
        resources: {}
        terminationMessagePath: /dev/termination-log
        terminationMessagePolicy: File
      dnsPolicy: ClusterFirst
      restartPolicy: Always
      schedulerName: default-scheduler
      securityContext: {}
      serviceAccount: authorino-authorino
      serviceAccountName: authorino-authorino
      terminationGracePeriodSeconds: 30
status:
  availableReplicas: 1
  conditions:
  - lastTransitionTime: "2025-01-09T17:46:38Z"
    lastUpdateTime: "2025-01-09T17:46:38Z"
    message: Deployment has minimum availability.
    reason: MinimumReplicasAvailable
    status: "True"
    type: Available
  - lastTransitionTime: "2025-01-09T17:46:27Z"
    lastUpdateTime: "2025-01-09T17:46:38Z"
    message: ReplicaSet "authorino-589494f86b" has successfully progressed.
    reason: NewReplicaSetAvailable
    status: "True"
    type: Progressing
  observedGeneration: 1
  readyReplicas: 1
  replicas: 1
  updatedReplicas: 1

/cc @guicassolato

[openshift-ci]

@bartoszmajsak: GitHub didn't allow me to request PR reviews from the following users: guicassolato.

Note that only opendatahub-io members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

I looked at the code in Authorino again (Kuadrant/authorino-operator#91) and it seems it should work, but there seems to be a bug, as after applying:

❯ kubectl apply  -f -<<EOF
apiVersion: operator.authorino.kuadrant.io/v1beta1
kind: Authorino
metadata:
 name: authorino
 labels:
   sidecar.istio.io/inject: "true"
spec:
 image: quay.io/kuadrant/authorino:latest
 replicas: 1
 clusterWide: true
 listener:
   tls:
     enabled: false
 oidcServer:
   tls:
     enabled: false
EOF

authorino.operator.authorino.kuadrant.io/authorino created

my deployment looks like that:

apiVersion: apps/v1
kind: Deployment
metadata:
 annotations:
   deployment.kubernetes.io/revision: "1"
 creationTimestamp: "2025-01-09T17:46:27Z"
 generation: 1
 labels:
   sidecar.istio.io/inject: "true"
 name: authorino
 namespace: default
 ownerReferences:
 - apiVersion: operator.authorino.kuadrant.io/v1beta1
   blockOwnerDeletion: true
   controller: true
   kind: Authorino
   name: authorino
   uid: 26155b7b-3ef0-4fb6-98d8-741a0c180f6f
 resourceVersion: "584"
 uid: f64c0bca-a799-4d15-b3e9-1c5018671b35
spec:
 progressDeadlineSeconds: 600
 replicas: 1
 revisionHistoryLimit: 10
 selector:
   matchLabels:
     authorino-resource: authorino
     control-plane: controller-manager
 strategy:
   rollingUpdate:
     maxSurge: 25%
     maxUnavailable: 25%
   type: RollingUpdate
 template:
   metadata:
     creationTimestamp: null
     labels:
       authorino-resource: authorino
       control-plane: controller-manager
   spec:
     containers:
     - image: quay.io/kuadrant/authorino:latest
       imagePullPolicy: Always
       name: authorino
       resources: {}
       terminationMessagePath: /dev/termination-log
       terminationMessagePolicy: File
     dnsPolicy: ClusterFirst
     restartPolicy: Always
     schedulerName: default-scheduler
     securityContext: {}
     serviceAccount: authorino-authorino
     serviceAccountName: authorino-authorino
     terminationGracePeriodSeconds: 30
status:
 availableReplicas: 1
 conditions:
 - lastTransitionTime: "2025-01-09T17:46:38Z"
   lastUpdateTime: "2025-01-09T17:46:38Z"
   message: Deployment has minimum availability.
   reason: MinimumReplicasAvailable
   status: "True"
   type: Available
 - lastTransitionTime: "2025-01-09T17:46:27Z"
   lastUpdateTime: "2025-01-09T17:46:38Z"
   message: ReplicaSet "authorino-589494f86b" has successfully progressed.
   reason: NewReplicaSetAvailable
   status: "True"
   type: Progressing
 observedGeneration: 1
 readyReplicas: 1
 replicas: 1
 updatedReplicas: 1

/cc @guicassolato

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-sigs/prow repository.

[zdtsw]

i will close my PR as deployment.injection.patch.tmpl should do the work

[bartoszmajsak]

FYI I opened PR with the fix in Authorino Operator Kuadrant/authorino-operator#236. Once this is merged and available in the version you rely on the aforementioned YAML patch can be entirely removed.

[dhirajsb]

@bartoszmajsak after your Authorino fix goes in, we still need to add this label to the Authorino CR, correct? That's why I appoved this PR.

[zdtsw]

@bartoszmajsak after your Authorino fix goes in, we still need to add this label to the Authorino CR, correct? That's why I appoved this PR.

i will wait for mentioned PR get into Authorino release first then make a new PR with this (add label) change and cleanup patch template in one go.

[bartoszmajsak]

@bartoszmajsak after your Authorino fix goes in, we still need to add this label to the Authorino CR, correct? That's why I appoved this PR.

But merging this now will be a source of confusion, as it has no effect on the behavior in the cluster. I think it's better to add it when it actually can be properly verified (together with removal of the current patchy approach).

@zdtsw it will also require removing FeatureTracker for existing RHOAI installation during upgrade.