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

Automatically add OIDC flags to API server when Auth-bundle is installed #3550

Closed
Tracked by #3202
vvondruska opened this issue Jul 4, 2024 · 6 comments · May be fixed by giantswarm/dex-operator#109
Closed
Tracked by #3202
Assignees

Comments

@vvondruska
Copy link

vvondruska commented Jul 4, 2024

Implement functionality to fulfil the following requirement:

  • have something that automatically patches the WC CR in the MC so that we don't need to do it manually but rather only need to terminate the node getting rid of copy-paste errors, indentation errors, ...
@github-project-automation github-project-automation bot moved this to Inbox 📥 in Roadmap Jul 4, 2024
@vvondruska vvondruska self-assigned this Jul 4, 2024
@vvondruska vvondruska moved this from Inbox 📥 to Up Next ➡️ in Roadmap Jul 4, 2024
@vvondruska vvondruska moved this from Up Next ➡️ to In Progress ⛏️ in Roadmap Jul 4, 2024
@vvondruska
Copy link
Author

Dex operator will be extended to patch the WC resources and add the OIDC flags unless they already exist.

@vvondruska
Copy link
Author

This feature will only be available in CAPI clusters.

Dex operator will check the KubeadmControlPlane CR to find out if the OIDC flags have already been set.
In case there are no OIDC flags in the CR, the operator will create a new configmap, add the flags there and reference the new configmap as an additional extra config in the cluster app.

Once the new cluster configuration is accepted and applied, the CP nodes are rolled automatically.
Therefore, this feature MUST be implemented as opt-in only.

Also, the implementation will cover this ticket and #3551.

@vvondruska
Copy link
Author

Added support for a new cluster.giantswarm.io/update-oidc-flags annotation in the Dex app. If set to true, it will add OIDC flags to the cluster configuration unless they already exist. Cluster configuration update will roll the nodes.

giantswarm/dex-operator#109

@vvondruska vvondruska linked a pull request Jul 22, 2024 that will close this issue
@vvondruska
Copy link
Author

Adding the flags works as expected now.
However, there is an open question about deletion:

When the Dex app is deleted from the cluster, the OIDC flags configured in the Kubeadm COntrol Plane will no longer be valid. However, if they are removed from the cluster, control plane nodes will be rolled, which may not always be desirable.

So, question is - in case Dex operator takes care of adding the OIDC flags to the cluster, can it also take care of their deletion when the Dex app is deleted? Or is it safer to keep the OIDC flags in the cluster and rely on the user to remove them manually?

@vvondruska
Copy link
Author

Adding OIDC flags may be tricky because they need to be applied to the cluster app. The preferred way to configure clusters is via gitops, which means that the source of truth for all configuration is/should be on the customer's side. If we make modifications to the cluster configuration on our side, it may be overwritten by the next change introduced by the customer. And even if it isn't, t may still create conflicts with the customer configuration.

Updating OIDC flags automatically is still considered somewhat valuable. It was suggested that we expose a flag in the cluster app configuration, and if it's enabled, the Auth bundle would be installed when the cluster is created and OIDC flags would be set.

@vvondruska
Copy link
Author

Closing this issue since automating the OIDC flags addition/removal by using Dex operator to modify the Cluster app configuration is not considered safe or desirable.

The alternative approach suggested above will be explored and a new issue will be created for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant