This repository has been archived by the owner on Oct 12, 2023. It is now read-only.
Adding support for whitelisting of user-defined managed identities #431
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
|
|
idanlevin
reviewed
Oct 23, 2019
idanlevin
reviewed
Oct 23, 2019
idanlevin
reviewed
Oct 23, 2019
kkmsft
reviewed
Oct 23, 2019
added 2 commits
October 24, 2019 12:05
kkmsft
reviewed
Oct 24, 2019
kkmsft
reviewed
Oct 24, 2019
docs/readmes/README.featureflags.md
Outdated
| > Available from 1.5.4 release | ||
|
|
||
| Aad-pod-identity has a new flag immutableUserMSIs which can be used to prevent deletetion of spcecified identitites from VM/VMSS. | ||
| The list is comma separated. |
There was a problem hiding this comment.
Can you please show a simple example - /subscription/00000000-000... like that.. you can check the main README for example of the user identity pattern.
There was a problem hiding this comment.
The example is added: Example: 00000000-0000-0000-0000-000000000000,11111111-1111-1111-1111-111111111111
amirschw
suggested changes
Oct 25, 2019
amirschw
reviewed
Oct 27, 2019
kkmsft
reviewed
Nov 1, 2019
test/e2e/aadpodidentity_test.go
Outdated
| @@ -133,6 +134,21 @@ var _ = Describe("Kubernetes cluster using aad-pod-identity", func() { | |||
| deleteAllIdentityValidator() | |||
| }) | |||
|
|
|||
| It("should not delete the Immutable Identity from vmss when the deployment is deleted", func() { | |||
| setUpIdentityAndDeployment(immutableIdentity, "", "1") | |||
There was a problem hiding this comment.
Ideally the e2e could:
- Assign the immutable identity explicitly to the vmss/vm
- Create an azure identity with the same immutable identity
- Run the identity validator check(ensure that a cycle has completed)
- Now the identity validator is deleted (so it goes through a cycle of deletion) .
- Check that count of azureassignedidentity is 0 (confirming that the reconcile cycle is complete)
- Check that the immutable identity is still present on the vm/vmss.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
kkmsft
reviewed
Nov 4, 2019
| @@ -98,7 +103,10 @@ func main() { | |||
| config.Burst = int(clientQPS) | |||
| glog.Infof("Client QPS set to: %v. Burst to: %v", config.QPS, config.Burst) | |||
|
|
|||
| micClient, err := mic.NewMICClient(cloudconfig, config, forceNamespaced, syncRetryDuration, &leaderElectionCfg, enableScaleFeatures, createDeleteBatch) | |||
| immutableUserMSIsList := strings.Split(immutableUserMSIs, ",") | |||
| glog.Infof("immutable identities are %v", immutableUserMSIsList) | |||
There was a problem hiding this comment.
nit0: Can you please make this conditional, If the parameter is given, then only print.
nit1: Can we have the first letter with caps.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Merged
statbit
pushed a commit
to adobe-platform/aad-pod-identity
that referenced
this pull request
Sep 30, 2021
…zure#431) * Adding support for whitelisting of user-defined managed identities * Fixing pull request comments * adding example for immutableUserMSIs flags readme * fixing rebase * improving helm chart to be more convenient * improving readme file * fixing remarks in Readme file * reverting go.sum changes * adding e2e test for immutable identity * refactoring immutable identity test * fixing e2e test for immutable identity
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Reason for Change:
In some cases a user-defined managed identity could be removed from the AAD
Issue Fixed:
This PR is adding whitelist of user defined managed identities what will never be removed from AAD
Notes for Reviewers:
The change is as following:
This feature is extremely important for migrating existing production clusters to aad-pod-identity.
In particular, all our existing deployments will use user-defined identity as one per team.
Reuse of the same identity to multiple pods caused the identity to be deleted from AAD when it still in use.
This change will solve the issue.
Best regards,
Dmitry