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

List expiration dates for 'mc admin user svcacct list' command #4576

Merged
merged 3 commits into from May 26, 2023

Conversation

kaankabalak
Copy link
Collaborator

@kaankabalak kaankabalak commented May 19, 2023

Description

This commit enables the user to list the expiration dates of the Service Accounts listed on the mc admin user svcacct list command.

Motivation and Context

As the user will be able to set the expiry of a Service Account on mc after PR #4570, it would also be practical for the users to see when they expire on the mc admin user svcacct list command.

Please merge this PR together with minio/madmin-go#195, minio/minio#17249 and minio/console#2824.

How to test this PR?

% ./mc admin user svcacct list ALIAS TARGET_ACCOUNT
   Access Key        | Expiry                       
5XF3ZHNZK6FBDWH9JMLX | 2023-06-24 07:00:00 +0000 UTC
F4V2BBUZSWY7UG96ED70 | 2023-12-24 18:00:00 +0000 UTC
FZVSEZ8NM9JRBEQZ7B8Q | no-expiry                    
HOXGL8ON3RG0IKYCHCUD | no-expiry                    
  • For the case where there's no service accounts present, also use the mc admin user svcacct list ALIAS TARGET_ACCOUNT and make sure that Service Accounts aren't listed.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Optimization (provides speedup with no functional changes)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Fixes a regression (If yes, please add commit-id or PR # here)
  • Unit tests added/updated
  • Internal documentation updated
  • Create a documentation update request here

@kaankabalak
Copy link
Collaborator Author

kaankabalak commented May 19, 2023

Tests seem to be failing as we are not pointing to the minio/madmin-go#195 branch, @kannappanr could you advise me on how we move forward in these cases? Do we temporarily add a replace directive on go.mod that points to the branch, so that we can see if tests pass or not?

Copy link
Contributor

@shtripat shtripat left a comment

Choose a reason for hiding this comment

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

LGTM. Verified with minio PR#17249

@kaankabalak
Copy link
Collaborator Author

Hi @harshavardhana, looks like we have 2 approving reviews, would it be possible to merge this and make a release so that I can update the packages for the other 2 PRs in minio/minio#17249 and minio/console#2824?

Copy link
Member

@vadmeste vadmeste left a comment

Choose a reason for hiding this comment

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

LGTM

@vadmeste
Copy link
Member

@kaankabalak in fact we usually make sure to merge the changes in the server side first, just easier to manage and avoid users opening new tickets (users are expected to have mc version more recent than server version, that is why we follow this model)

@vadmeste
Copy link
Member

Hi @harshavardhana, looks like we have 2 approving reviews, would it be possible to merge this and make a release so that I can update the packages for the other 2 PRs in minio/minio#17249 and minio/console#2824?

Oh you mean you are trying to resolve the dependency issue?

@harshavardhana harshavardhana merged commit 0fac0e0 into minio:master May 26, 2023
5 checks passed
@kaankabalak
Copy link
Collaborator Author

Hi @harshavardhana, looks like we have 2 approving reviews, would it be possible to merge this and make a release so that I can update the packages for the other 2 PRs in minio/minio#17249 and minio/console#2824?

Oh you mean you are trying to resolve the dependency issue?

Yes 👍

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.

None yet

4 participants