ARO-6446 :: Create managed identities when not provided to az aro create#4789
ARO-6446 :: Create managed identities when not provided to az aro create#4789komidore64 wants to merge 2 commits intomasterfrom
az aro create#4789Conversation
komidore64
requested review from
alcasim,
bennerv,
cadenmarchese,
hawkowl,
hlipsig,
jharrington22,
kevinobriendotca,
kimorris27,
mociarain,
mrWinston,
rogbas,
sankur-codes,
tiguelu,
tsatam,
tuxerrante,
ventifus,
wanghaoran1988 and
yjst2012
as code owners
|
|
||
| if enable_managed_identity is True: | ||
| if enable_managed_identity: | ||
| if not platform_workload_identities and not mi_user_assigned: |
There was a problem hiding this comment.
Could a check be added here for to see if version is set to something other than None and allow --version to still be optional when not using managed identities?
There was a problem hiding this comment.
Updated to provide clearer validation for the user, and only require --version if --enable-managed-identity is passed when --assign-platform-workload-identity and --assign-cluster-identity are absent.
|
|
||
| if enable_managed_identity is True: | ||
| if enable_managed_identity: | ||
| if not platform_workload_identities and not mi_user_assigned: |
There was a problem hiding this comment.
If only one of --mi-user-assigned or --assign-platform-wi is provided, this falls through the auto-create condition and hits dict(None) (TypeError) or {None: {}}. Should the and be or, or add validation for partial input?
There was a problem hiding this comment.
and is the correct logic in this instance.
We want to accept any and all user-provided identities -- correct or otherwise -- or auto-create identities if neither are provided.
Related to your question, I want to add additional output to _validators.py:317-325 for more descriptive user feedback than a python stacktrace.
There was a problem hiding this comment.
Updated to provide clearer validatation.
|
I received feedback/input from some folks in Slack that it's too much of a breaking change to require The desired flag combinations of
|
komidore64
force-pushed
the
adprice/aro-6446-identities-with-cluster
branch
2 times, most recently
from
7eede32 to
0b07912
Compare
.. if the user creates a managed identity cluster and does not provide the necessary identities as command arguments. Only require `--version` if `--enable-managed-identity` is given while `--assign-platform-workload-identity` and `--assign-cluster-identity` are absent. Ref: https://redhat.atlassian.net/browse/ARO-6446
komidore64
force-pushed
the
adprice/aro-6446-identities-with-cluster
branch
from
0b07912 to
22c306a
Compare
3 participants
Which issue this PR addresses:
Fixes ARO-6446
What this PR does / why we need it:
Adds managed identity creation into
az aro createfor users that wish to have a managed-identity cluster create identities and role assignments for them.BREAKING CHANGE (CLI): OpenShift version (--version X.Y.Z) is now a required flag when creating a cluster withaz aro create.OpenShift version is only required if
--enable-managed-identityis passed while--assign-platform-workload-identityand--assign-cluster-identityare absent.Test plan for issue:
I smoke tested the creation of a few clusters using
az aro create --enable-managed-identity.Is there any documentation that needs to be updated for this PR?
I included an example command for enabling managed identities in the example help output.
How do you know this will function as expected in production?
These changes use currently existing endpoints and functionality.