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

Provide validation for identityId with Azure pod identity in TriggerAuthentication #4528

Closed
tomkerkhove opened this issue May 5, 2023 · 10 comments
Assignees

Comments

@tomkerkhove
Copy link
Member

Provide validation for identityId with Azure pod identity in TriggerAuthentication.

I've seen a scenario where an empty application ID was specified and it was not blocked, most probably because we see it as the default?

@tomkerkhove tomkerkhove transferred this issue from kedacore/keda-docs May 5, 2023
@JorTurFer
Copy link
Member

Empty value is the default value, so we don't have any good way to difference between empty by error or just not provided.
Which is the scenario that you want to avoid? I mean, if a user provides an empty clientID, the default clientID will be used. We could validate the input checking if it's a GUID or not, but I'm not sure if that's useful either

@tomkerkhove
Copy link
Member Author

Can we distinguish empty from nil? Imagine someone wants to use identityId but the automation/configuration contains a bug and causes to pass an empty ID instead it will pas (ie tokenization failure, etc).

@JorTurFer
Copy link
Member

I'm not totally sure if we can, because an empty string isn't nil, is "". I'd need to check if we are using a pointer here. I'll review it later on

@tomkerkhove
Copy link
Member Author

I'm not totally sure if we can, because an empty string isn't nil, is ""

Well that is my question actually, can we use nil as the default value instead of ""? Because if we can, that means we can provide validation and block creation of resources where "" was specified.

@JorTurFer
Copy link
Member

JorTurFer commented May 9, 2023

Currently, the default value is "" and not nil. We'd need to modify the code to use pointers as part of the validation process changes

@tomkerkhove
Copy link
Member Author

That sounds like a viable thing to do allowing us to validate the input, what do you think?

It's just an idea, I don't want to push this though :)

@JorTurFer
Copy link
Member

I think that we can give a try

@tomkerkhove tomkerkhove self-assigned this May 10, 2023
@tomkerkhove tomkerkhove moved this from To Triage to To Do in Roadmap - KEDA Core May 10, 2023
@tomkerkhove
Copy link
Member Author

Let's see if I can do this :)

@tomkerkhove
Copy link
Member Author

@SpiritZhou is working on this

@tomkerkhove tomkerkhove moved this from To Do to In Review in Roadmap - KEDA Core Jul 6, 2023
@SpiritZhou SpiritZhou self-assigned this Jul 24, 2023
@tomkerkhove tomkerkhove moved this from In Review to Done in Roadmap - KEDA Core Aug 29, 2023
@tomkerkhove
Copy link
Member Author

Thanks @SpiritZhou !

@github-project-automation github-project-automation bot moved this from Done to Ready To Ship in Roadmap - KEDA Core Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

3 participants