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

Update Service Acct fns based on new ListServiceAccounts type #2824

Merged
merged 8 commits into from
May 30, 2023

Conversation

kaankabalak
Copy link
Contributor

@kaankabalak kaankabalak commented May 19, 2023

As per minio/mc#4576, in order to be able to list Service Accounts with their expiration dates on mc, we would need to update Service Account functions in Console.

Please merge this PR before minio/minio#17249 as it is a dependency.

@kaankabalak
Copy link
Contributor Author

Tests seem to be failing as we are not pointing to the correct branches, @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 while we run the tests?

@cesnietor
Copy link
Collaborator

Tests seem to be failing as we are not pointing to the correct branches, @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 while we run the tests?

Since this is a dependency, we would need to first upgrade mc @kaankabalak and then do a release so that we can update it here in console.

@kaankabalak kaankabalak force-pushed the svcacct-property branch 4 times, most recently from 39a37de to 34dc889 Compare May 23, 2023 05:03
@kaankabalak
Copy link
Contributor Author

Will address the integration test failure

@kaankabalak
Copy link
Contributor Author

Will address the integration test failure

Fixed the restapi test, however the integration test is failing as we are using the minio/minio image that doesn't contain the change located at minio/minio#17249.

vadmeste
vadmeste previously approved these changes May 26, 2023
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

@kaankabalak
Copy link
Contributor Author

I have just updated the PR to use the untagged mc version which includes minio/mc#4576, we can now merge the PR if it's okay to do so 👍

@dvaldivia
Copy link
Collaborator

@kaankabalak I also see minio/minio#17249 is not merged yet

@harshavardhana
Copy link
Member

@kaankabalak I also see minio/minio#17249 is not merged yet

It needs to be merged here first since this is a server dependency.

@kaankabalak
Copy link
Contributor Author

kaankabalak commented May 29, 2023

@kaankabalak I also see minio/minio#17249 is not merged yet

Hi @dvaldivia, as @harshavardhana said, this is a dependency for minio/minio#17249, therefore we can merge this PR first 👍

I have also edited the first comment of this PR accordingly.

@harshavardhana
Copy link
Member

A failing test due to the server not having the support for the new data structure

&{500 Internal Server Error 500 HTTP/1.1 1 1 map[Content-Length:[199] Content-Type:[application/json] Date:[Tue, 30 May 2023 17:06:01 GMT] Server:[MinIO Console] Vary:[Accept-Encoding] X-Content-Type-Options:[nosniff] X-Frame-Options:[DENY] X-Xss-Protection:[1; mode=block]] 0xc000b96120 199 [] true false map[] 0xc00120e900 <nil>} <nil>
StatusCode: 500
    users_test.go:763: 
        	Error Trace:	/home/runner/work/console/console/integration/users_test.go:763
        	Error:      	Not equal: 
        	            	expected: 200
        	            	actual  : 500
        	Test:       	TestCreateServiceAccountForUser
        	Messages:   	Http Response: {"code":500,"detailedMessage":"json: cannot unmarshal string into Go struct field ListServiceAccountsResp.accounts of type madmin.ServiceAccountInfo","message":"an error occurred, please try again"}
    users_test.go:769: 
        	Error Trace:	/home/runner/work/console/console/integration/users_test.go:769
        	Error:      	Not equal: 
        	            	expected: 214
        	            	actual  : 40
        	Test:       	TestCreateServiceAccountForUser
        	Messages:   	Http Response: {"code":500,"detailedMessage":"json: cannot unmarshal string into Go struct field ListServiceAccountsResp.accounts of type madmin.ServiceAccountInfo","message":"an error occurred, please try again"}
--- FAIL: TestCreateServiceAccountForUser (0.55s)

@kaankabalak
Copy link
Contributor Author

A failing test due to the server not having the support for the new data structure

&{500 Internal Server Error 500 HTTP/1.1 1 1 map[Content-Length:[199] Content-Type:[application/json] Date:[Tue, 30 May 2023 17:06:01 GMT] Server:[MinIO Console] Vary:[Accept-Encoding] X-Content-Type-Options:[nosniff] X-Frame-Options:[DENY] X-Xss-Protection:[1; mode=block]] 0xc000b96120 199 [] true false map[] 0xc00120e900 <nil>} <nil>
StatusCode: 500
    users_test.go:763: 
        	Error Trace:	/home/runner/work/console/console/integration/users_test.go:763
        	Error:      	Not equal: 
        	            	expected: 200
        	            	actual  : 500
        	Test:       	TestCreateServiceAccountForUser
        	Messages:   	Http Response: {"code":500,"detailedMessage":"json: cannot unmarshal string into Go struct field ListServiceAccountsResp.accounts of type madmin.ServiceAccountInfo","message":"an error occurred, please try again"}
    users_test.go:769: 
        	Error Trace:	/home/runner/work/console/console/integration/users_test.go:769
        	Error:      	Not equal: 
        	            	expected: 214
        	            	actual  : 40
        	Test:       	TestCreateServiceAccountForUser
        	Messages:   	Http Response: {"code":500,"detailedMessage":"json: cannot unmarshal string into Go struct field ListServiceAccountsResp.accounts of type madmin.ServiceAccountInfo","message":"an error occurred, please try again"}
--- FAIL: TestCreateServiceAccountForUser (0.55s)

Hi @harshavardhana, as stated here, the test is failing because it is using the latest minio which doesn't contain the changes in minio/minio#17249

@cesnietor cesnietor merged commit 5a77054 into minio:master May 30, 2023
@kaankabalak kaankabalak deleted the svcacct-property branch May 30, 2023 17:41
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.

5 participants