fix: Move K8scel driver from framework #3570
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ module github.com/open-policy-agent/gatekeeper/v3 | |
|
|
||
| go 1.22.0 | ||
|
|
||
| replace github.com/open-policy-agent/frameworks/constraint v0.0.0-20240927180816-0f64229c5539 => github.com/abhipatnala/frameworks/constraint v0.0.0-20241010172729-61e15952c5c5 | ||
|
|
||
| require ( | ||
| cloud.google.com/go/trace v1.10.11 | ||
| github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.44.0 | ||
|
|
@@ -15,7 +17,8 @@ require ( | |
| github.com/google/uuid v1.6.0 | ||
| github.com/onsi/gomega v1.34.1 | ||
| github.com/open-policy-agent/cert-controller v0.11.0 | ||
| github.com/open-policy-agent/frameworks/constraint v0.0.0-20241007142041-e84361fed758 | ||
|
There was a problem hiding this comment. @abhipatnala this should point to e78c8abd754aa12538eb21ee0c145cbe5465569d |
||
| github.com/open-policy-agent/opa v0.68.0 | ||
| github.com/pkg/errors v0.9.1 | ||
| github.com/prometheus/client_golang v1.20.4 | ||
| github.com/spf13/cobra v1.8.1 | ||
|
|
@@ -30,7 +33,7 @@ require ( | |
| go.opentelemetry.io/otel/sdk/metric v1.28.0 | ||
| go.uber.org/automaxprocs v1.5.3 | ||
| go.uber.org/zap v1.26.0 | ||
| golang.org/x/net v0.29.0 | ||
| golang.org/x/oauth2 v0.21.0 | ||
| golang.org/x/sync v0.8.0 | ||
| golang.org/x/time v0.6.0 | ||
|
|
@@ -41,6 +44,7 @@ require ( | |
| k8s.io/api v0.30.5 | ||
| k8s.io/apiextensions-apiserver v0.30.5 | ||
| k8s.io/apimachinery v0.30.5 | ||
| k8s.io/apiserver v0.30.5 | ||
| k8s.io/client-go v0.30.5 | ||
| k8s.io/klog/v2 v2.120.1 | ||
| k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 | ||
|
|
@@ -113,7 +117,6 @@ require ( | |
| github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect | ||
| github.com/modern-go/reflect2 v1.0.2 // indirect | ||
| github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect | ||
| github.com/opencontainers/go-digest v1.0.0 // indirect | ||
| github.com/opencontainers/image-spec v1.1.0 // indirect | ||
| github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect | ||
|
|
@@ -138,18 +141,17 @@ require ( | |
| go.opentelemetry.io/proto/otlp v1.3.1 // indirect | ||
| go.uber.org/atomic v1.11.0 // indirect | ||
| go.uber.org/multierr v1.11.0 // indirect | ||
| golang.org/x/crypto v0.27.0 // indirect | ||
| golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect | ||
| golang.org/x/sys v0.25.0 // indirect | ||
| golang.org/x/term v0.24.0 // indirect | ||
| golang.org/x/text v0.18.0 // indirect | ||
| gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect | ||
| google.golang.org/api v0.189.0 // indirect | ||
| google.golang.org/genproto v0.0.0-20240722135656-d784300faade // indirect | ||
| google.golang.org/genproto/googleapis/api v0.0.0-20240722135656-d784300faade // indirect | ||
| google.golang.org/genproto/googleapis/rpc v0.0.0-20240722135656-d784300faade // indirect | ||
| gopkg.in/inf.v0 v0.9.1 // indirect | ||
| k8s.io/component-base v0.30.5 // indirect | ||
| k8s.io/kube-openapi v0.0.0-20240430033511-f0e62f92d13f // indirect | ||
| sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.29.0 // indirect | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There was a problem hiding this comment.
There was a problem hiding this comment. We should probably just wait for the scheme.Convert() bug to be fixed, as that's the "proper K8s" way to convert across versions. In general, core libraries should deal only with unversioned representations. This has the added advantage of reducing merge conflicts with your PR. There was a problem hiding this comment. I'm ok with leaving it as is. but can we add a TODO comment for the bug and open an issue so we can come back to it later to clean it up? @JaydipGabani There was a problem hiding this comment. I added the TODO - and had opened an issue - kubernetes/kubernetes#126582This is the response I got "Conversion is only defined to the internal "hub" API type, not directly from one external version to another, and is only done by the api server, not by clients." I will follow up with api-server to see if this is something that can be fixed. There was a problem hiding this comment. hmm then sounds like its not a bug. not a blocker for this PR tho. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,12 +27,12 @@ import ( | |
| "github.com/go-logr/logr" | ||
| "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates/v1beta1" | ||
| constraintclient "github.com/open-policy-agent/frameworks/constraint/pkg/client" | ||
| "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" | ||
| constraintstatusv1beta1 "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/config/process" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/constraintstatus" | ||
| celSchema "github.com/open-policy-agent/gatekeeper/v3/pkg/drivers/k8scel/schema" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/drivers/k8scel/transform" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/metrics" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/operations" | ||
|
|
@@ -49,8 +49,6 @@ import ( | |
| "k8s.io/apimachinery/pkg/runtime" | ||
| "k8s.io/apimachinery/pkg/runtime/schema" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| "k8s.io/utils/ptr" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| "sigs.k8s.io/controller-runtime/pkg/client/apiutil" | ||
|
|
@@ -75,11 +73,12 @@ var ( | |
| ErrValidatingAdmissionPolicyAPIDisabled = errors.New("ValidatingAdmissionPolicy API is not enabled") | ||
| ErrVAPConditionsNotSatisfied = errors.New("Conditions are not satisfied to generate ValidatingAdmissionPolicy and ValidatingAdmissionPolicyBinding") | ||
| ) | ||
|
|
||
| // var vapMux sync.RWMutex | ||
|
|
||
| // var VapAPIEnabled *bool | ||
|
|
||
| // var GroupVersion *schema.GroupVersion | ||
|
There was a problem hiding this comment. pls remove commented code. |
||
|
|
||
| type Adder struct { | ||
| CFClient *constraintclient.Client | ||
|
|
@@ -317,7 +316,7 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R | |
| isAPIEnabled := false | ||
| var groupVersion *schema.GroupVersion | ||
| if generateVAPB { | ||
| isAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&log) | ||
| } | ||
| if generateVAPB { | ||
| if !isAPIEnabled { | ||
|
|
@@ -348,7 +347,7 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R | |
| r.log.Info("constraint controller", "generateVAPB", generateVAPB) | ||
| // generate vapbinding resources | ||
| if generateVAPB && groupVersion != nil { | ||
| currentVapBinding, err := transform.VapBindingForVersion(*groupVersion) | ||
| if err != nil { | ||
| return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") | ||
| } | ||
|
|
@@ -389,7 +388,7 @@ func (r *ReconcileConstraint) Reconcile(ctx context.Context, request reconcile.R | |
| // do not generate vapbinding resources | ||
| // remove if exists | ||
| if !generateVAPB && groupVersion != nil { | ||
| currentVapBinding, err := transform.VapBindingForVersion(*groupVersion) | ||
| if err != nil { | ||
| return reconcile.Result{}, r.reportErrorOnConstraintStatus(ctx, status, err, "could not get ValidatingAdmissionPolicyBinding API version") | ||
| } | ||
|
|
@@ -620,78 +619,6 @@ func (c *ConstraintsCache) reportTotalConstraints(ctx context.Context, reporter | |
| } | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. I agree with @maxsmythe on
Let's not move there two methods. |
||
| func getRunTimeVAPBinding(gvk *schema.GroupVersion, transformedVapBinding *admissionregistrationv1beta1.ValidatingAdmissionPolicyBinding, currentVapBinding client.Object) (client.Object, error) { | ||
| if currentVapBinding == nil { | ||
| if gvk.Version == "v1" { | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There was a problem hiding this comment.
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,13 +24,13 @@ import ( | |
|
|
||
| "github.com/open-policy-agent/frameworks/constraint/pkg/apis/templates/v1beta1" | ||
| constraintclient "github.com/open-policy-agent/frameworks/constraint/pkg/client" | ||
| "github.com/open-policy-agent/frameworks/constraint/pkg/core/templates" | ||
| statusv1beta1 "github.com/open-policy-agent/gatekeeper/v3/apis/status/v1beta1" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/constraint" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/constraintstatus" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/controller/constrainttemplatestatus" | ||
| celSchema "github.com/open-policy-agent/gatekeeper/v3/pkg/drivers/k8scel/schema" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/drivers/k8scel/transform" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/logging" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/metrics" | ||
| "github.com/open-policy-agent/gatekeeper/v3/pkg/operations" | ||
|
|
@@ -479,7 +479,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate( | |
| isVapAPIEnabled := false | ||
| var groupVersion *schema.GroupVersion | ||
| if generateVap { | ||
| isVapAPIEnabled, groupVersion = transform.IsVapAPIEnabled(&logger) | ||
| } | ||
| logger.Info("isVapAPIEnabled", "isVapAPIEnabled", isVapAPIEnabled) | ||
| logger.Info("groupVersion", "groupVersion", groupVersion) | ||
|
|
@@ -490,7 +490,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate( | |
| } | ||
| // generating vap resources | ||
| if generateVap && isVapAPIEnabled && groupVersion != nil { | ||
| currentVap, err := transform.VapForVersion(groupVersion) | ||
| if err != nil { | ||
| logger.Error(err, "error getting vap object with respective groupVersion") | ||
| err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not get VAP with runtime group version", status, err) | ||
|
|
@@ -545,7 +545,7 @@ func (r *ReconcileConstraintTemplate) handleUpdate( | |
| // do not generate vap resources | ||
| // remove if exists | ||
| if !generateVap && isVapAPIEnabled && groupVersion != nil { | ||
| currentVap, err := transform.VapForVersion(groupVersion) | ||
| if err != nil { | ||
| logger.Error(err, "error getting vap object with respective groupVersion") | ||
| err := r.reportErrorOnCTStatus(ctx, ErrCreateCode, "Could not get VAP with correct group version", status, err) | ||
|
|
@@ -747,17 +747,6 @@ func makeGvk(kind string) schema.GroupVersionKind { | |
| } | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. I agree with @maxsmythe on
Let's not move there two methods. |
||
| func getRunTimeVAP(gvk *schema.GroupVersion, transformedVap *admissionregistrationv1beta1.ValidatingAdmissionPolicy, currentVap client.Object) (client.Object, error) { | ||
| if currentVap == nil { | ||
| if gvk.Version == "v1" { | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@abhipatnala can you pls open a PR on framework as well so we can review and get it merged before merging this one? thanks!
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.
@ritazh Here is the Pr for removing cel driver from framework: open-policy-agent/frameworks#486
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.
@abhipatnala PTAL and remove the go.mod override 😄