-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
add Azure config dump #15134
base: devel
Are you sure you want to change the base?
add Azure config dump #15134
Conversation
@@ -174,6 +187,22 @@ def handle(self, *args, **options): | |||
else: | |||
data.append({f"LDAP_{awx_ldap_name}_missing_fields": ldap_missing_fields}) | |||
|
|||
# dump AZURE settings | |||
awx_azure_settings = self.get_awx_azure_settings() | |||
awx_azure_enabled, azure_missing_fields = self.is_enabled(awx_azure_settings, self.DAB_AZURE_AUTHENTICATOR_KEYS) |
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.
I see that is_enabled
is a method that is already established prior to this PR, but I'm concerned that it treats an explicit None
the same as a missing key/value. Worse, now that I'm thinking of it, any explicit value that evaluates as false-like. This is IMO a bug waiting to happen.
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.
The way I'd deal with it:
- establish a module variable
MISSING = object()
- condition becomes
if required and settings.get(key, MISSING) is MISSING:
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.
See my comments.
SUMMARY
dump_auth_config will now also dump Azure AD config data.
ISSUE TYPE
COMPONENT NAME
AWX VERSION
ADDITIONAL INFORMATION
if Azure AD is not configured
if Azure AD is configured