fix: Move K8scel driver from framework #3570
There are no files selected for viewing
|
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. |
| 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,6 @@ var ( | |
| ErrValidatingAdmissionPolicyAPIDisabled = errors.New("ValidatingAdmissionPolicy API is not enabled") | ||
| ErrVAPConditionsNotSatisfied = errors.New("Conditions are not satisfied to generate ValidatingAdmissionPolicy and ValidatingAdmissionPolicyBinding") | ||
| ) | ||
|
|
||
| type Adder struct { | ||
| CFClient *constraintclient.Client | ||
|
|
@@ -317,7 +310,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 { | ||
|
|
@@ -620,67 +613,6 @@ func (c *ConstraintsCache) reportTotalConstraints(ctx context.Context, reporter | |
| } | ||
| } | ||
|
|
||
| func vapBindingForVersion(gvk schema.GroupVersion) (client.Object, error) { | ||
|
There was a problem hiding this comment. I agree with @maxsmythe on
Let's not move there two methods. |
||
| switch gvk.Version { | ||
| case "v1": | ||
|
|
||
|
There was a problem hiding this comment.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package k8scel | ||
|
|
||
| type Arg func(*Driver) error | ||
|
|
||
| // GatherStats starts collecting various stats around the | ||
| // underlying engine's calls. | ||
| func GatherStats() Arg { | ||
| return func(driver *Driver) error { | ||
| driver.gatherStats = true | ||
|
|
||
| return nil | ||
| } | ||
| } |
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 this should point to e78c8abd754aa12538eb21ee0c145cbe5465569d