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

OIDC: remove deprecated variable #1722

Merged

Conversation

cniackz
Copy link
Contributor

@cniackz cniackz commented Aug 13, 2023

Objectives:

  • To remove deprecated var from our example: MINIO_IDENTITY_OPENID_REDIRECT_URI
  • And also from other spots we located after.

Related:

Documentation:

Testing:

  1. Tested on existing IDP configuration, it works!.
  2. Tested when configured via UI, it works!
  3. Tested when configured via CLI, it works!
  4. Tested when configured via Tenant Spec, it works!

@cniackz cniackz force-pushed the adjust-helm-chart-for-deprecated-var-2 branch 2 times, most recently from 841e18b to ac3ea5e Compare August 13, 2023 18:41
@cniackz cniackz changed the title remove deprecated var remove deprecated variable Aug 13, 2023
@cniackz cniackz changed the title remove deprecated variable remove deprecated IDP variable Aug 13, 2023
@cniackz cniackz changed the title remove deprecated IDP variable OIDC: remove deprecated variable Aug 13, 2023
@cniackz cniackz self-assigned this Aug 13, 2023
jiuker
jiuker previously requested changes Aug 14, 2023
Copy link
Contributor

@jiuker jiuker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tenantConfigurationENV["MINIO_IDENTITY_OPENID_REDIRECT_URI"] = callbackURL

"MINIO_IDENTITY_OPENID_REDIRECT_URI",

Still have this

@cniackz
Copy link
Contributor Author

cniackz commented Aug 14, 2023

Thank you very much @jiuker I will also address those spots and re-test.

@cniackz cniackz force-pushed the adjust-helm-chart-for-deprecated-var-2 branch from ac3ea5e to 1028f27 Compare August 14, 2023 14:59
@feorlen
Copy link
Contributor

feorlen commented Aug 14, 2023

A question: does this mean identity_openid redirect_uri no longer works either?

https://min.io/docs/minio/linux/reference/minio-mc-admin/mc-admin-config.html#mc-conf.identity_openid.redirect_uri

@cniackz
Copy link
Contributor Author

cniackz commented Aug 14, 2023

Hello @feorlen, identity_openid redirect_uri is deprecated, we don't need to provide this value anymore. If provided, it is ignored and not used.

@cniackz cniackz force-pushed the adjust-helm-chart-for-deprecated-var-2 branch 2 times, most recently from bffb6f1 to 579cce7 Compare August 14, 2023 15:20
@cniackz cniackz force-pushed the adjust-helm-chart-for-deprecated-var-2 branch from 579cce7 to 2b00ca4 Compare August 14, 2023 15:20
@cniackz
Copy link
Contributor Author

cniackz commented Aug 14, 2023

I have removed more spots. Tests have to pass then I ask for another round of reviews.

@cniackz cniackz requested a review from jiuker August 14, 2023 15:50
@cniackz cniackz dismissed jiuker’s stale review August 14, 2023 15:50

The requested changes were completed, new review is required!.

Copy link
Contributor

@jiuker jiuker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pjuarezd pjuarezd merged commit 85fe853 into minio:master Aug 14, 2023
24 checks passed
@cniackz cniackz deleted the adjust-helm-chart-for-deprecated-var-2 branch August 14, 2023 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants