Skip to content
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

fix: Upgrade azserect client for AZURE #430

Closed
wants to merge 7 commits into from

Conversation

jiuker
Copy link

@jiuker jiuker commented Jan 11, 2024

Upgrade azserect client
How to test:

export ClientID=UUID
export EndPoint=url
export Secret=secret
export TenantID=tid
export ManagedIdentityClientID=micd
go test github.com/minio/kes/internal/keystore/azure

Once the verification is successful, I remove the redundant code
fix #261

internal/keystore/azure/key-vault-error.go Show resolved Hide resolved
internal/keystore/azure/key-vault-error.go Outdated Show resolved Hide resolved
internal/keystore/azure/key-vault-error.go Outdated Show resolved Hide resolved
internal/keystore/azure/key-vault.go Outdated Show resolved Hide resolved
internal/keystore/azure/key-vault.go Outdated Show resolved Hide resolved
@jiuker jiuker requested a review from aead January 12, 2024 09:38
internal/keystore/azure/client.go Outdated Show resolved Hide resolved
@bh4t
Copy link

bh4t commented Jan 23, 2024

Hey @aead can you please review this when you get a chance?

Copy link
Member

@harshavardhana harshavardhana left a comment

Choose a reason for hiding this comment

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

Only minor comment, rest of them looks fine. We need to get rid of the old code @jiuker

internal/keystore/azure/key-vault-error.go Outdated Show resolved Hide resolved
@vadmeste
Copy link
Member

@jiuker did you get the chance to test this with Azure ? let me know if you need any help in Azure

@jiuker
Copy link
Author

jiuker commented Feb 1, 2024

@jiuker did you get the chance to test this with Azure ? let me know if you need any help in Azure

Yeah. Pre commit have test code. I have tested it. @vadmeste

@harshavardhana
Copy link
Member

@aead we need to get this merged for a new kes release, so that we can move to a supported SDKs. Can you PTAL ?

@bh4t
Copy link

bh4t commented Feb 13, 2024

@aead can you please review this?

@bh4t bh4t added the bug fix label Feb 13, 2024
harshavardhana
harshavardhana previously approved these changes Feb 13, 2024
@harshavardhana
Copy link
Member

harshavardhana commented Feb 20, 2024

@aead we need to migrate to new SDK previous one is unmaintained now.. its a security risk.

…client

# Conflicts:
#	internal/keystore/azure/client.go
@harshavardhana
Copy link
Member

Checking back again on this @aead let me know if you could look at this PR.

@harshavardhana
Copy link
Member

#459 supersedes this PR, closing this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azure: migrate to a non-deprecated SDK
6 participants