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

Adding TLSSecurityProfile to CRD #399

Merged
merged 1 commit into from
Aug 10, 2022
Merged

Adding TLSSecurityProfile to CRD #399

merged 1 commit into from
Aug 10, 2022

Conversation

borod108
Copy link
Contributor

As part of supporting Openshift cluster-wide crypto policy,
adding required fields to SSP CRD.

Signed-off-by: borod108 boris.od@gmail.com

None

@kubevirt-bot kubevirt-bot added the release-note-none Denotes a PR that doesn't merit a release note. label Jul 13, 2022
@kubevirt-bot kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/L labels Jul 13, 2022
@akrejcir
Copy link
Collaborator

Are we going to use this new field in a future PR? If yes, it would make sense to do the API change in the same PR as usage.

@borod108
Copy link
Contributor Author

borod108 commented Aug 4, 2022

@tiraboschi as Anrdrej requested, we will post this PR again with all the code for supporting the crypto policy, but can you just take a look and confirm the API is right?

@borod108
Copy link
Contributor Author

borod108 commented Aug 4, 2022

@akrejcir can we merge this, so that #408 is cleaner?

@akrejcir
Copy link
Collaborator

akrejcir commented Aug 4, 2022

Why not close this PR and only keep #408 ?
Anyway I'm taking a look...

akrejcir
akrejcir reviewed Aug 4, 2022
View reviewed changes
go.mod Outdated Show resolved Hide resolved
api/go.mod Outdated Show resolved Hide resolved
@tiraboschi
Copy link
Member

@tiraboschi as Anrdrej requested, we will post this PR again with all the code for supporting the crypto policy, but can you just take a look and confirm the API is right?

Yes, the API looks fine

@borod108
Copy link
Contributor Author

borod108 commented Aug 7, 2022

Just because it will look cleaner - this PR generates a lot of code, it is easier in my mind to have those PRs separate. I would prefer it this way, but it is up to you.

@akrejcir
Copy link
Collaborator

akrejcir commented Aug 8, 2022

Ok. Can you rebase? Then it can be merged.

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2022
@kubevirt-bot kubevirt-bot added size/L and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL labels Aug 9, 2022
akrejcir
akrejcir requested changes Aug 9, 2022
View reviewed changes
api/go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
akrejcir
akrejcir reviewed Aug 9, 2022
View reviewed changes
Copy link
Collaborator

@akrejcir akrejcir left a comment

Choose a reason for hiding this comment

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

I checked-out this code locally and run:

cd api
go mod vendor
go mod tidy

cd ..
go mod vendor
go mod tidy

And the file api/go.sum has some changes. Can you commit them?

go.mod Outdated Show resolved Hide resolved
@borod108
Adding TLSSecurityProfile to CRD
fdb3be0 
As part of supporting Openshift cluster-wide crypto policy,
adding required fields to SSP CRD.

Signed-off-by: borod108 <boris.od@gmail.com>
@sonarcloud
Copy link

sonarcloud bot commented Aug 10, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@borod108
Copy link
Contributor Author

/retest

@akrejcir
Copy link
Collaborator

/lgmt
/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akrejcir

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 10, 2022
@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2022
@kubevirt-bot kubevirt-bot merged commit 46995b6 into kubevirt:master Aug 10, 2022
@akrejcir
Copy link
Collaborator

akrejcir commented Sep 5, 2022

/cherry-pick release-v0.16

@kubevirt-bot
Copy link
Contributor

@akrejcir: new pull request created: #413

In response to this:

/cherry-pick release-v0.16

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kubevirt-bot kubevirt-bot mentioned this pull request Sep 5, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akrejcir akrejcir akrejcir requested changes

@ksimon1 ksimon1 ksimon1 approved these changes

Assignees

@akrejcir akrejcir

@ksimon1 ksimon1

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@borod108 @akrejcir @tiraboschi @kubevirt-bot @ksimon1