Skip to content

migrate role assignment cleanup from bash to Go#25

Open
nojnhuh wants to merge 1 commit intoAzure:masterfrom
nojnhuh:roleassignments
Open

migrate role assignment cleanup from bash to Go#25
nojnhuh wants to merge 1 commit intoAzure:masterfrom
nojnhuh:roleassignments

Conversation

@nojnhuh
Copy link
Copy Markdown
Member

@nojnhuh nojnhuh commented Mar 12, 2025

This PR migrates the role assignment cleanup logic driven by the az CLI and bash from #22 to Go.

Comment on lines +1 to +3
FROM alpine:3.21
COPY bin/rg-cleanup /usr/local/bin
ENTRYPOINT [ "rg-cleanup" ]
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same as before #22, but with a bumped alpine from 3.18 to the latest minor version.

Comment on lines 1 to +3
IMAGE_REGISTRY ?= k8sprowcomm.azurecr.io
IMAGE_NAME := rg-cleanup
IMAGE_VERSION ?= v0.4.6
IMAGE_VERSION ?= v0.4.7
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface of the container should be the same since it keeps the --role-assignments flag, so I opted to only bump the patch version here.

github.com/Azure/azure-sdk-for-go v36.2.0+incompatible
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.6.0
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.3.0
github.com/Azure/azure-sdk-for-go/sdk/azcore v1.15.0
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should take a pass at updating these dependencies sometime, but these versions have been working for me locally for now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking the same, but its probably safer to make that a follow-on PR.

Copy link
Copy Markdown
Member

@mboersma mboersma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lgtm, but let me test it IRL.

resourceGroupClient, err := armresources.NewResourceGroupsClient(o.subscriptionID, chain, &options)
idReq := serviceprincipals.NewGetByIdsPostRequestBody()
idReq.SetIds(assignedPrincipalIDs)
idRes, err := graph.ServicePrincipals().GetByIds().PostAsGetByIdsPostResponse(ctx, idReq, &serviceprincipals.GetByIdsRequestBuilderPostRequestConfiguration{})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I give PostAsGetByIdsPostResponse 1 out of 5 stars as a function name. :-p

@jsturtevant
Copy link
Copy Markdown

This lgtm, but let me test it IRL.

did this work?

@mboersma
Copy link
Copy Markdown
Member

did this work?

It didn't seem to--the container was still running after 20 minutes, so I cancelled it, not realizing that means we get no logs. I didn't have much time to test, so I reverted to our known-working version. I'd like to run the rg-cleanup binary locally to see if I can get more info but probably can't get to it until Wednesday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants