-
Notifications
You must be signed in to change notification settings - Fork 8
feat: dexop - operator to manage Oauth2 clients in Dex #876
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
Conversation
2f506f3 to
a239090
Compare
| type SecretManager struct { | ||
| } | ||
|
|
||
| func (s SecretManager) readSecret(r *ClientReconciler, ctx context.Context, name, namespace string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any specific reasons for not using pointer here func (s *SecretManager)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The methods in SecretManager do not alter the struct's state, and because SecretManager is currently an empty struct, there's no need for a pointer receiver. A value receiver suffices for stateless behavior and avoids unnecessary complexity of pointer management.
|
|
||
| toolchain go1.23.6 | ||
|
|
||
| require ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to run go mod tidy
current go.mod is missing some packages that are used.
Like: github.com/sethvargo/go-password
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch,thanks! fixed in rebase
cardoe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall seems like what we're aiming to achieve.
Up till now all tests required the Dex server to be already running on the developer's machine. It was also assumed that the server will be preconfigured for the gRPC communication. This required the appropriate certificates to be generated, configured in Dex and copied over to the project directory. This commit removes that requirement altogether by automatically starting "dex serve" with appropriate configuration and shutting it down when the tests complete.
grpc.Dial deprecation will be done in separate commit in case w need to revert.
This deploys basic version of the operator.
This changes how the automatically generated Kubernetes Secret looks like: - the `secret` key was renamed to `client-secret` - the `client-id` key is now populated - the `issuer` is populated Caveat: The Issuer information can be obtained from Dex dynamically only starting from newest version (2.42) of Dex. To avoid triggering failed discovery, the same information can be provided as a command line argument.
abhimanyu003
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This pull request introduces a new operator that enables dynamic management of OAuth2 clients in Dex using standard Kubernetes objects.
Currently, creating a Client involves several manual steps:
This process is not ideal, as it imposes a significant administrative burden and security risks due to the use of environment variables. Moreover, updating passwords becomes challenging.
By deploying the operator from this pull request, we can simplify this process significantly. The operator will handle generating secrets and updating Dex configurations for the listed Clients, eliminating the need for manual intervention. Additionally, it leverages dynamic configuration of Dex, allowing us to make changes without requiring a restart. As an added security benefit, the actual password values will never leave the cluster, reducing the risk of exposure and improving overall security.
Key benefits of this change include reduced administrative burden, improved security, and easier password updates.
Note
To the reviewer:
Part of this PR is boiler plate that is automatically generated by operator-sdk. This can be ignored or skimmed through:
config/folder is automatically generated with exception ofconfig/samplesdist/contains an autogenerated YAML for non-helm installsMakefileis autogenerated with adjustments to theIMAGE_TAG_BASEonly