Added FP Check for validation #3335
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carvalhe
requested review from
jewzaam,
bennerv,
hawkowl,
rogbas,
petrkotas,
jharrington22,
cblecker,
cadenmarchese,
UlrichSchlueter,
s-amann,
SudoBrendan,
Shivkumar13,
yjst2012,
anshulvermapatel and
hlipsig
as code owners
|
Looks good to me, but we will have to test this to validate. As discussed, let's use the provided test tenant to simulate and see whether we get a correct error message. |
|
On a second thought, I think we should adjust the error message for invalid token that the service principal couldn't be found too. |
nwnt
reviewed
Dec 28, 2023
tsatam
reviewed
Jan 10, 2024
s-fairchild
requested changes
Jan 15, 2024
SudoBrendan
approved these changes
Jan 16, 2024
s-fairchild
approved these changes
Jan 16, 2024
s-fairchild
approved these changes
Jan 16, 2024
Closed
mociarain
pushed a commit
that referenced
this pull request
Feb 8, 2024
* Added FP Check for validation * Cloud Err to surface level backend for SP issue
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue this PR addresses:
Fixes https://issues.redhat.com/browse/ARO-4734
What this PR does / why we need it:
Users are getting "Internal Server Error" when they have a deleted ServicePrincipal, and we want to give them a better and more detailed response. We noticed that the SP validation checks happen that would catch this and return the right error, but FP doesn't so we are doing the same validation step for FP as well.
Test plan for issue:
Looking into ways to test this as it would require removing a SP on this branch in an INT env.