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

Use a secret instead of a configmap for alpha config #153

Merged

Conversation

mkjpryor
Copy link
Contributor

@mkjpryor mkjpryor commented Jun 7, 2023

This PR addresses the concerns in #104, namely that using the alpha config requires storing the client secret in a configmap, by moving the whole alpha config to a secret.

Once we have a better way of locating secrets, this can be changed.

@mkjpryor
Copy link
Contributor Author

mkjpryor commented Jun 7, 2023

Note that the existing behaviour of alphaConfig.existingConfig pointing to an existing configmap is retained for backwards compatibility.

@mkjpryor
Copy link
Contributor Author

mkjpryor commented Jun 7, 2023

Note that the patch version was bumped rather than the minor version as:

  1. The change is backwards compatible
  2. I consider storing secret information in a configmap to be a bug

@pierluigilenoci
Copy link
Contributor

@JoelSpeed, could you please take a look at this?

@pierluigilenoci
Copy link
Contributor

@mkjpryor, could you please rebase?

@mkjpryor mkjpryor force-pushed the feature/alpha-config-secret branch from dd508b1 to 040029a Compare June 7, 2023 16:10
@mkjpryor
Copy link
Contributor Author

mkjpryor commented Jun 7, 2023

@pierluigilenoci rebase done

@jessebot
Copy link

jessebot commented Jul 8, 2023

Thanks for your work here! Could I ask if there's any blockers here? This feature would be awesome!

@pierluigilenoci
Copy link
Contributor

@jessebot I was waiting for a review from @JoelSpeed.
Ref: #104 (comment)

@pierluigilenoci pierluigilenoci merged commit 081d2a3 into oauth2-proxy:main Jul 10, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants