Lay groundwork for cluster MSI usage #3757
Conversation
kimorris27
force-pushed
the
kimorris27/ARO-4360-cluster-msi-changes
branch
2 times, most recently
from
c95aad2 to
81de97a
Compare
|
/azp run ci, e2e |
kimorris27
requested review from
jewzaam,
bennerv,
hawkowl,
rogbas,
petrkotas,
jharrington22,
cblecker,
cadenmarchese,
UlrichSchlueter,
SudoBrendan,
yjst2012,
jaitaiwan,
anshulvermapatel,
hlipsig,
tiguelu,
SrinivasAtmakuri,
mociarain,
AldoFusterTurpin and
tsatam
as code owners
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
Stuck a hold label on this because I remembered that I had to comment out the |
…dation bootstrap step
- In newly added Azure clients, return struct types instead of interface types - Move cluster MSI certificate deletion to be after Azure resource deletion for safety just in case cx continues to use cluster that is in Failed/Deleting provisioning state
kimorris27
dismissed stale reviews from cadenmarchese and rajdeepc2792
via
179d711
kimorris27
force-pushed
the
kimorris27/ARO-4360-cluster-msi-changes
branch
from
071f769 to
179d711
Compare
|
/azp run ci, e2e |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| // Note that we are assuming that all of the platform MIs are in the same subscription. | ||
| pwiResourceId, err := arm.ParseResourceID(m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[0].ResourceID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| subId := pwiResourceId.SubscriptionID |
There was a problem hiding this comment.
Not a blocking change, but I'm curious why we're getting the subscriptionID off of the first platform workload identity, if we're assuming that all identities are in the same subscription as the cluster.
Would we not be able to just access the subscriptionID directly off of the clusterdoc (e.g. m.subscriptionDoc.ID), as I had done for in-cluster secrets generation here:
There was a problem hiding this comment.
I guess I didn't think about it that much, since we're assuming we'll get the same ID from either place. Since I'll be opening a follow-up PR anyway, how about I switch to using the subscription doc there?
| t.Run(tt.name, func(t *testing.T) { | ||
| controller := gomock.NewController(t) | ||
| defer controller.Finish() | ||
|
|
There was a problem hiding this comment.
Is this mock controller unused?
There was a problem hiding this comment.
Oops, it is. Added to my todo list for the follow-up PR.
Which issue this PR addresses:
https://issues.redhat.com/browse/ARO-4360
What this PR does / why we need it:
This PR lays the groundwork for later usage of cluster MSI by:
IsLocalDevelopmentMode()is not sufficient)Test plan for issue:
Is there any documentation that needs to be updated for this PR?
I don't think so, but let me know if I missed anything.
How do you know this will function as expected in production?
MIWI will be tested extensively before the feature is released.