fix: verify DeploymentConfig API when installing keycloak using Template #1329
Conversation
Signed-off-by: Chetan Banavikalmutt <[email protected]>
There was a problem hiding this comment.
One question I have for this PR:
My concern with this PR is that it is sending new OpenShift 4.15 clusters down an untested path: currently, (I suspect) the vast vast majority of OpenShift users would have DeploymentConfig configured, and thus when keyclock was enabled, it would use the template-based approach.
Very very few users would be using the non-Template, Ingress-based approach: basically only argocd-operator users on non-OpenShift.
However, with this PR, we are now sending all those new OpenShift 4.15 users (with cluster that defaults to not having this API) down this mostly untested code path.
All those users would switch from using the Template API-based codepath that is commonly used, to the ingress-based codepath which is rarely used. (And anything that is rarely used is a lot less likely to work)
For example, if you don't have the required APIs installed, we will no longer install keycloak from the official openshift image on registry.redhat.com: registry.redhat.io/rh-sso-7/sso76-openshift-rhel8
Instead, we install keycloak from quay.io/keycloak/keycloak:
// ArgoCDKeycloakImage is the default Keycloak Image used for the non-openshift platforms when not specified.
ArgoCDKeycloakImage = "quay.io/keycloak/keycloak"
As an example of the kind of issues we might encounter, the quay.io/keycloak version hasn't been updated in 2 years, whereas the openshift tag was updated 7 months ago.
An alternative to the approach taken in this PR would be just to disable the keycloak functionality entirely if the user doesn't have the required APIs, and output a message telling the user to install those PRs.
Signed-off-by: Chetan Banavikalmutt <[email protected]>
|
@jgwest That's a valid concern. I agree that the Operator should prefer the official image for Keycloak on OCP clusters. I've updated the PR to log and return an error if the user selects Keycloak but the DeploymentConfig API is absent. I also had to check if the Template API is present to ensure it is an OCP cluster. This should prevent new users from installing the unofficial image. As a long-term solution, we should update the Template to use Deployment instead. |
Signed-off-by: Chetan Banavikalmutt <[email protected]>
|
@jgwest @iam-veeramalla This PR might be redundant if we plan to move away from Templates. Until then we can accept this as a temporary solution so users don't see errors when the DeploymentConfig is disabled. WDYT? |
|
Merging as no objections were raised. |
…ate (argoproj-labs#1329) * fix: verify DeploymentConfig API when installing keycloak using Template Signed-off-by: Chetan Banavikalmutt <[email protected]> * return an error if the DeploymentConfig API is not present on OCP Signed-off-by: Chetan Banavikalmutt <[email protected]> * log when the Keycloak instance can't be managed using Template Signed-off-by: Chetan Banavikalmutt <[email protected]> --------- Signed-off-by: Chetan Banavikalmutt <[email protected]>
What type of PR is this?
/kind bug
What does this PR do / why we need it:
Users have the option to disable DeploymentConfig API on their OCP clusters. In such cases, the operator shouldn't watch the DeploymentConfig APIs and shouldn't install Keycloak using Template.
https://docs.openshift.com/container-platform/4.14/installing/cluster-capabilities.html#deployment-config-capability_cluster-capabilities
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes https://issues.redhat.com/browse/GITOPS-4489
How to test changes / Special notes to the reviewer: