-
Notifications
You must be signed in to change notification settings - Fork 4
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
Set sync setting in config automatically #18
Set sync setting in config automatically #18
Conversation
b631778
to
78bdff3
Compare
e9e01ab
to
6c21610
Compare
6c21610
to
0770055
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow! This is quite the PR, @yiraeChristineKim! I need to look at the E2E test and look through the Gatekeeper docs to figure out the pieces here and the details on what it's doing, but it'll be a powerful addition to the operator!
I have some comments/questions that I thought I'd leave in the meantime.
8d05256
to
c774fb1
Compare
b00bd96
to
060cb6d
Compare
060cb6d
to
beac12b
Compare
f390bac
to
d5614ff
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great test coverage. This PR is getting really close!
d5614ff
to
2fd5697
Compare
2fd5697
to
7dec403
Compare
controllers/cps_controller_helper.go
Outdated
|
||
constraintMatchKinds, _, err := unstructured.NestedSlice(constraint.Object, "spec", "match", "kinds") | ||
if err != nil { | ||
r.Log.V(1).Info("There are no provided kinds in the Contsraint", "constraintName:", constraintName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r.Log.V(1).Info("There are no provided kinds in the Contsraint", "constraintName:", constraintName) | |
r.Log.V(1).Info("There are no provided kinds in the Constraint", "constraintName:", constraintName) |
7dec403
to
1a2fb9f
Compare
controllers/cps_controller_helper.go
Outdated
cpsMgr, err := ctrl.NewManager(r.KubeConfig, ctrl.Options{ | ||
Scheme: r.Scheme, | ||
Metrics: server.Options{ | ||
BindAddress: ":0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BindAddress: ":0", | |
BindAddress: "0", |
Signed-off-by: Yi Rae Kim <[email protected]>
ffaab24
to
c49fddf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mprahl, yiraeChristineKim 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 |
/cherrypick release-3.14 |
@yiraeChristineKim: new pull request created: #32 In response to this:
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. |
/cherrypick release-3.11 |
@yiraeChristineKim: #18 failed to apply on top of branch "release-3.11":
In response to this:
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. |
Description: The gatekeeper operator exposes a setting in the CRD under audit named auditFromCache. By default this cache is Disabled. If you set it to Enabled, you break your constraints because the cache requires additional settings in the CRD configs.config.gatekeeper.sh for the sync details.
The problem (my opinion here) is that if we expose the ability to enable the cache in the operator we must also expose the ability to configure the cache with the sync details.
Ref: https://issues.redhat.com/browse/ACM-7065