Skip to content

Add check for missing IdentityProvider configuration. #12

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

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

ggivo
Copy link
Contributor

@ggivo ggivo commented Feb 7, 2025

It can happen that client ID/secret are not set for some reason (e.g read from env variables but because of misconfiguration null values are used). In such case, identityProviderConfig will remain null and generate NPE when later used.

Introducing an additional check that at least one of the expected configuration settings is applied before invoking build()

@ggivo ggivo requested a review from atakavci February 7, 2025 11:01
@ggivo ggivo marked this pull request as draft February 7, 2025 11:07
Copy link
Collaborator

@atakavci atakavci left a comment

Choose a reason for hiding this comment

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

@ggivo this makes sense. thanks for dealing with that..
how about failing checks,, are they related ?

@ggivo
Copy link
Contributor Author

ggivo commented Feb 13, 2025

@atakavci
I think it is a generic issue in the pipeline that we need to look at separately.
Since automatic bump PRs are also failing with the same errors (#13)

@atakavci
Copy link
Collaborator

right,, this is due to missing secrets in forks.

about spellcheck, looks like i didnt pay enough attention to that in #10
do you mind adding these

Entra
EntraID
Gradle
Jedis
Reauthentication

to wordlist ?? or i could.

@atakavci
Copy link
Collaborator

i ll come up with a plan for missing secrets later.

@ggivo
Copy link
Contributor Author

ggivo commented Feb 13, 2025

@atakavci

do you mind adding these

Will do.

@ggivo ggivo requested a review from atakavci February 13, 2025 09:15
Copy link
Collaborator

@atakavci atakavci left a comment

Choose a reason for hiding this comment

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

Thanks !

@ggivo ggivo marked this pull request as ready for review February 13, 2025 13:10
@ggivo ggivo merged commit e51a97b into redis:main Feb 13, 2025
2 of 3 checks passed
@ggivo ggivo deleted the verify-config-provided branch February 13, 2025 13:10
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.

2 participants