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

chore: reworks authorino istio proxy injection patch #1097

Conversation

bartoszmajsak
Copy link
Contributor

Description

Instead of performing patching of Authorino deployment as part of PostConditions hook, it is now a Feature on its own.

As a result, we no longer need the ApplyManifest mehtod for the Feature struct. This function was created solely to apply a single manifest as an Action and was used only for this specific use case. With the dedicated feature, a deployment patch can now be defined as a regular manifest source and included as a part of the Apply phase.

How Has This Been Tested?

I tested both the upgrade path and regular deployment on ROSA (hence serviceMesh.auth.audiences entry in the DSCI below)

Prerequisite: DSCI with Service Mesh enabled

apiVersion: dscinitialization.opendatahub.io/v1
kind: DSCInitialization
metadata:
  name: default-dsci
spec:
  applicationsNamespace: opendatahub
  monitoring:
    managementState: Removed
    namespace: opendatahub-monitoring
  trustedCABundle:
      managementState: Managed
  serviceMesh:
    managementState: Managed
    auth:
      audiences:
      - https://rh-oidc.s3.us-east-1.amazonaws.com/27bd6cg0vs7nn08mue83fbof94dj4m9a

Upgrade path

  • deploy the latest operator (e.g. incubation)
  • apply DSCI
  • notice Authorino deployed with istio-proxy enabled
  • the following feature trackers should be persisted in the cluster
❯ oc get featuretrackers -o name
featuretracker.features.opendatahub.io/opendatahub-enable-proxy-injection-in-authorino-deployment
featuretracker.features.opendatahub.io/opendatahub-mesh-control-plane-creation
featuretracker.features.opendatahub.io/opendatahub-mesh-control-plane-external-authz
featuretracker.features.opendatahub.io/opendatahub-mesh-metrics-collection
featuretracker.features.opendatahub.io/opendatahub-mesh-shared-configmap
  • deploy operator from this PR (quay.io/maistra-dev/opendatahub-operator:dev-patch-authorino-deployment-as-a-feature)
  • trigger reconcile (e.g. change monitoring ns to some other value assuming it's in Removed management state)
  • observe no change in the cluster setup (all resources are up-to-date), but new Feature tracker is created:
❯ oc get featuretrackers -o name
featuretracker.features.opendatahub.io/opendatahub-enable-proxy-injection-in-authorino-deployment
featuretracker.features.opendatahub.io/opendatahub-mesh-control-plane-creation
featuretracker.features.opendatahub.io/opendatahub-mesh-control-plane-external-authz
featuretracker.features.opendatahub.io/opendatahub-mesh-metrics-collection
featuretracker.features.opendatahub.io/opendatahub-mesh-shared-configmap

Warning

Simple removal of DSCI might trigger reconcile error for Trusted CA bundle. Needs #1095 to be merged first.

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

Instead of performing patching of Authorino deployment as part of
`PostConditions` hook, it is now a `Feature` on its own.

As a result, we no longer need the `ApplyManifest` mehtod for the `Feature` struct.

This function was created solely to apply a single manifest as an `Action` and
was used only for this specific use case. With the dedicated feature, a deployment
patch can now be defined as a regular manifest source and included as part of the Apply phase.
Copy link

openshift-ci bot commented Jul 4, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cam-garrison, zdtsw

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci bot added the approved label Jul 4, 2024
@zdtsw
Copy link
Member

zdtsw commented Jul 4, 2024

/test opendatahub-operator-e2e

@openshift-merge-bot openshift-merge-bot bot merged commit 13d833e into opendatahub-io:incubation Jul 4, 2024
8 checks passed
VaishnaviHire pushed a commit to VaishnaviHire/opendatahub-operator that referenced this pull request Jul 24, 2024
Instead of performing patching of Authorino deployment as part of
`PostConditions` hook, it is now a `Feature` on its own.

As a result, we no longer need the `ApplyManifest` mehtod for the `Feature` struct.

This function was created solely to apply a single manifest as an `Action` and
was used only for this specific use case. With the dedicated feature, a deployment
patch can now be defined as a regular manifest source and included as part of the Apply phase.

(cherry picked from commit 13d833e)
VaishnaviHire pushed a commit to VaishnaviHire/opendatahub-operator that referenced this pull request Jul 24, 2024
Instead of performing patching of Authorino deployment as part of
`PostConditions` hook, it is now a `Feature` on its own.

As a result, we no longer need the `ApplyManifest` mehtod for the `Feature` struct.

This function was created solely to apply a single manifest as an `Action` and
was used only for this specific use case. With the dedicated feature, a deployment
patch can now be defined as a regular manifest source and included as part of the Apply phase.

(cherry picked from commit 13d833e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants