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.

Sign up for GitHub

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

Jump to bottom

Update versions and fix KES Test #1457

Merged
merged 3 commits into from
Feb 28, 2023
Merged

Update versions and fix KES Test #1457

merged 3 commits into from
Feb 28, 2023

Conversation

cniackz
Copy link
Contributor

@cniackz cniackz commented Feb 17, 2023 •
edited
Loading

Objective:

  • New Go Version, from 1.19 to 1.20
  • Also couple of extra changes for failing tests like KES due to missing policies.

@cniackz cniackz force-pushed the fix-vulnerability branch 3 times, most recently from 5cdda2e to 3f3b0f2 Compare February 17, 2023 22:39
@cniackz cniackz changed the title fix vulnerability Fix vulnerability Feb 17, 2023
@cniackz cniackz force-pushed the fix-vulnerability branch 2 times, most recently from 4eb1096 to f3c9ebd Compare February 17, 2023 22:49
@cniackz cniackz self-assigned this Feb 17, 2023
@cniackz cniackz force-pushed the fix-vulnerability branch 6 times, most recently from 3716664 to 15dfc13 Compare February 17, 2023 23:30
@cniackz cniackz mentioned this pull request Feb 18, 2023
Closed
@cniackz
Copy link
Contributor Author

cniackz commented Feb 18, 2023 •
edited
Loading

I don't understand why KES Test is failing here:

Added `kestest` successfully.
Handling connection for 9000
mc: <ERROR> Failed to get status information: not authorized: insufficient permissions.
/home/runner/work/operator/operator/testing/console-tenant+kes.sh: cannot mc admin kms key status kestest --insecure
Deleting cluster "kind" ...
E0218 00:18:03.099823   47397 portforward.go:234] lost connection to pod
E0218 00:18:03.105574   41874 portforward.go:234] lost connection to pod
Error: Process completed with exit code 111.

If Added `kestest` successfully. was added, why we are failing to get the status: cannot mc admin kms key status kestest --insecure Deleting cluster "kind" ...

I am debugging this issue: mc: <ERROR> Failed to get status information: not authorized: insufficient permissions.

@dvaldivia
Copy link
Collaborator

tenant-kes test is failing due to mc: <ERROR> Failed to get status information: not authorized: insufficient permissions. error, so I we need to look into it

@allanrogerr
Copy link
Contributor

@cniackz @dvaldivia Let us know when to re-review

@cniackz
Copy link
Contributor Author

cniackz commented Feb 22, 2023

@allanrogerr I am still trying to understand why KES test is failing here, @kannappanr gave me some ideas, so I will be trying to fix this test prior review but will let you know when it is ready for review, thanks for checking on this! 👍

@cniackz
Copy link
Contributor Author

cniackz commented Feb 22, 2023

@dvaldivia, @kannappanr gave me the answer already last Saturday, it was due to missing Policy /v1/status, I did the fix already in this PR that will allow us to get the status again, if it pass, I will ping you via Slack to review this together.

@cniackz
Copy link
Contributor Author

cniackz commented Feb 22, 2023

image

@cniackz
Copy link
Contributor Author

cniackz commented Feb 22, 2023

We should be removing Prometheus and Log test since we are no longer supporting those features. But that can be done later on!.

@cniackz
Copy link
Contributor Author

cniackz commented Feb 22, 2023

Guys, this is ready to review all tests are now passing:

Screenshot 2023-02-22 at 6 50 05 PM

pjuarezd
pjuarezd previously approved these changes Feb 22, 2023
View reviewed changes
Praveenrajmani
Praveenrajmani previously approved these changes Feb 23, 2023
View reviewed changes
Copy link
Contributor

@Praveenrajmani Praveenrajmani left a comment

Choose a reason for hiding this comment

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

LGTM

testing/common.sh Show resolved Hide resolved
allanrogerr
allanrogerr previously approved these changes Feb 23, 2023
View reviewed changes
@pjuarezd pjuarezd mentioned this pull request Feb 27, 2023
Merged
reivaj05
reivaj05 previously approved these changes Feb 27, 2023
View reviewed changes
@pjuarezd
Copy link
Member

This PR is a nice well needed refresher, on top of it: can we modify this PR to keep tests for 1.19.x and 1.20.x @cniackz?

Motivations:

  • Keep checking on 1.19.x for the near future, when other projects like minio, minio-go, madmin-go, etc go 1.20.x jump to 1.20.x we are ready.
  • 1.19.6 and 1.20.1 solve a lot of CVE's, would be optimal test/build over the latest bugfix releases
  • I hope this and other projects are moving soon to 1.20.x

@pjuarezd pjuarezd dismissed stale reviews from reivaj05, allanrogerr, Praveenrajmani, and themself via a3e83a5 February 28, 2023 00:42
@cniackz cniackz changed the title Fix vulnerability Update versions and fix KES Test Feb 28, 2023
@cniackz cniackz mentioned this pull request Feb 28, 2023
Merged
@dvaldivia dvaldivia merged commit fdacf35 into minio:master Feb 28, 2023
pjuarezd pushed a commit to pjuarezd/operator that referenced this pull request Mar 2, 2023
@cniackz @pjuarezd
Update versions and fix KES Test (minio#1457)
c5f719d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@allanrogerr allanrogerr allanrogerr approved these changes

@Praveenrajmani Praveenrajmani Praveenrajmani approved these changes

@pjuarezd pjuarezd pjuarezd approved these changes

@dvaldivia dvaldivia Awaiting requested review from dvaldivia

@harshavardhana harshavardhana Awaiting requested review from harshavardhana

@reivaj05 reivaj05 Awaiting requested review from reivaj05

Assignees

@cniackz cniackz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@cniackz @dvaldivia @allanrogerr @pjuarezd @Praveenrajmani @reivaj05