-
Notifications
You must be signed in to change notification settings - Fork 149
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
api/vmauth: introduce unauthorizedUserAccess field at spec #1186
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
Previously for unauthorized_user config section of vmauth. Operator used two configuration fields: `unauthorizedAccessConfig` and inlined fields from `VMUserOptions`. This behaviour doesn't aligh with configuration file supported by vmauth. It also incorrectly exposed fields from `VMUserOptions` at `spec`. Which could mislead users, since `spec.default_urls` could be treated as global config option for vmauth, but in fact, it can only be used at `unauthorized_user` section. This commit replaces both fields with the new field `unauthorizedUserAccess`. It combines both config options - `url_map` and `VMUserOptions`. Replaced fields marked as deprecated and will be removed at `v1.0` operator API release. Also `VMauth` now properly validates `unauthorized_user` related configuration and returns proper error to the user, instead of crashing `vmauth` container in runtime. Related issues: - #1168 - #1169 Signed-off-by: f41gh7 <[email protected]>
Haleygo
reviewed
Dec 12, 2024
@@ -59,6 +62,24 @@ func (r *VMAuth) sanityCheck() error { | |||
r.Spec.ExternalConfig.SecretRef.Name, r.Spec.ExternalConfig.SecretRef.Key) | |||
} | |||
} | |||
|
|||
if len(r.Spec.UnauthorizedAccessConfig) > 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can also add a check for not having both UnauthorizedAccessConfig
and UnauthorizedUserAccess
specified.
Co-authored-by: Hui Wang <[email protected]>
Signed-off-by: f41gh7 <[email protected]>
Signed-off-by: f41gh7 <[email protected]>
api/operator/v1beta1/vmauth_types.go
Outdated
return nil | ||
} | ||
|
||
// UnauthorizedAccessConfigURLMap defines |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
// UnauthorizedAccessConfigURLMap defines | |
// UnauthorizedAccessConfigURLMap defines unauthorized_user section configuration for vmauth |
AndrewChubatiuk
previously approved these changes
Dec 12, 2024
Signed-off-by: f41gh7 <[email protected]>
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.
Previously for unauthorized_user config section of vmauth. Operator used two configuration fields:
unauthorizedAccessConfig
and inlined fields fromVMUserOptions
. This behaviour doesn't aligh with configuration file supportedby vmauth. It also incorrectly exposed fields from
VMUserOptions
atspec
.Which could mislead users, since
spec.default_urls
could be treated as global config option for vmauth, but in fact, it can only be used atunauthorized_user
section.This commit replaces both fields with the new field
unauthorizedUserAccess
.It combines both config options -
url_map
andVMUserOptions
. Replaced fields marked as deprecated and will be removed atv1.0
operator API release.Also
VMauth
now properly validatesunauthorized_user
related configuration and returns proper error to the user, instead of crashingvmauth
container in runtime.Related issues:
url_prefix
orurl_map
#1169