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.

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

feat: add type to policy CRD #1079

Merged
merged 6 commits into from
Sep 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/unversioned/policy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import (
type PolicySpec struct {
// Important: Run "make" to regenerate code after modifying this file

// Type of the polocy
binbin-li marked this conversation as resolved.
Show resolved Hide resolved
Type string `json:"type,omitempty"`

// Parameters for this policy
Parameters runtime.RawExtension `json:"parameters,omitempty"`
}
Expand Down
3 changes: 3 additions & 0 deletions api/v1alpha1/policy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ import (
type PolicySpec struct {
// Important: Run "make" to regenerate code after modifying this file

// Type of the polocy
Type string `json:"type,omitempty"`

// +kubebuilder:pruning:PreserveUnknownFields
// Parameters for this policy
Parameters runtime.RawExtension `json:"parameters,omitempty"`
Expand Down
2 changes: 2 additions & 0 deletions api/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions charts/ratify/crds/policy-customresourcedefinition.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
Expand Down Expand Up @@ -40,6 +41,9 @@ spec:
description: Parameters for this policy
type: object
x-kubernetes-preserve-unknown-fields: true
type:
description: Type of the polocy
type: string
type: object
status:
description: PolicyStatus defines the observed state of Policy
Expand Down
3 changes: 3 additions & 0 deletions config/crd/bases/config.ratify.deislabs.io_policies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ spec:
description: Parameters for this policy
type: object
x-kubernetes-preserve-unknown-fields: true
type:
description: Type of the polocy
type: string
type: object
status:
description: PolicyStatus defines the observed state of Policy
Expand Down
3 changes: 2 additions & 1 deletion config/samples/policy/config_v1alpha1_policy_json.yaml
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
apiVersion: config.ratify.deislabs.io/v1alpha1
kind: Policy
metadata:
name: "configpolicy"
name: "ratify-policy"
spec:
type: "configpolicy"
parameters:
artifactVerificationPolicies:
default: "all"
3 changes: 2 additions & 1 deletion config/samples/policy/config_v1alpha1_policy_rego.yaml
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
apiVersion: config.ratify.deislabs.io/v1alpha1
kind: Policy
metadata:
name: "regopolicy"
name: "ratify-policy"
spec:
type: "regopolicy"
parameters:
passthroughEnabled: false
policy: |
Expand Down
13 changes: 8 additions & 5 deletions docs/reference/crds/policies.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,24 @@ View more CRD samples [here](../../../config/samples/policy). The `metadata.name
apiVersion: config.ratify.deislabs.io/v1alpha1
kind: Policy
metadata:
name: "configpolicy"
akashsinghal marked this conversation as resolved.
Show resolved Hide resolved
name: "ratify-policy"
spec:
type: required. Type of the policy.
parameters: required. Parameters specific to this policy
```
There would be exactly one CR existing in the cluster. That means if users apply a new CR, the old one would be replaced.
Ratify would only accept a policy setting `metadata.name` as `ratify-policy` since there is at most one active policy for Ratify to use. That means if users apply a new CR with different `metadata.name`, Ratify will not update the existing policy.
akashsinghal marked this conversation as resolved.
Show resolved Hide resolved

Note: `metadata.name` MUST be `configpolicy` or `regopolicy` per the usage.
Note: `spec.name` MUST be `configpolicy` or `regopolicy` per the usage.

## configpolicy
Sample spec:
```yml
apiVersion: config.ratify.deislabs.io/v1alpha1
kind: Policy
metadata:
name: "configpolicy"
name: "ratify-policy"
spec:
type: "configpolicy"
parameters:
artifactVerificationPolicies:
"application/vnd.cncf.notary.signature": "any"
Expand All @@ -38,8 +40,9 @@ Sample spec:
apiVersion: config.ratify.deislabs.io/v1alpha1
kind: Policy
metadata:
name: "regopolicy"
name: "ratify-policy"
spec:
type: "regopolicy"
parameters:
passthroughEnabled: false
policy: |
Expand Down
38 changes: 11 additions & 27 deletions pkg/controllers/policy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
"sigs.k8s.io/controller-runtime/pkg/client"
)

const ratifyPolicy = "ratify-policy"

// PolicyReconciler reconciles a Policy object
type PolicyReconciler struct {
client.Client
Expand Down Expand Up @@ -74,33 +76,15 @@
return ctrl.Result{}, client.IgnoreNotFound(err)
}

if err := policyAddOrReplace(policy.Spec, resource); err != nil {
policyLogger.Error("unable to create policy from policy crd: ", err)
return ctrl.Result{}, err
if resource != ratifyPolicy {
return ctrl.Result{}, nil

Check warning on line 80 in pkg/controllers/policy_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/policy_controller.go#L79-L80

Added lines #L79 - L80 were not covered by tests
binbin-li marked this conversation as resolved.
Show resolved Hide resolved
}

// List all policies in the same namespace.
policyList := &configv1alpha1.PolicyList{}
if err := r.List(ctx, policyList, client.InNamespace(req.Namespace)); err != nil {
policyLogger.Error("failed to list Policies: ", err)
if err := policyAddOrReplace(policy.Spec); err != nil {
policyLogger.Error("unable to create policy from policy crd: ", err)

Check warning on line 84 in pkg/controllers/policy_controller.go

View check run for this annotation

Codecov / codecov/patch

pkg/controllers/policy_controller.go#L83-L84

Added lines #L83 - L84 were not covered by tests
return ctrl.Result{}, err
}

// Delete all policies except the current one.
for _, item := range policyList.Items {
item := item
if item.Name != resource {
policyLogger.Infof("Deleting policy %s", item.Name)
err := r.Delete(ctx, &item)
if err != nil {
policyLogger.Error("failed to delete Policy: ", err)
return ctrl.Result{}, err
}
policyLogger.Info("Deleted policy", "name", item.Name)
}
}

// returning empty result and no error to indicate we’ve successfully reconciled this object
return ctrl.Result{}, nil
}

Expand All @@ -111,19 +95,19 @@
Complete(r)
}

func policyAddOrReplace(spec configv1alpha1.PolicySpec, policyName string) error {
policyEnforcer, err := specToPolicyEnforcer(spec, policyName)
func policyAddOrReplace(spec configv1alpha1.PolicySpec) error {
policyEnforcer, err := specToPolicyEnforcer(spec)
if err != nil {
return fmt.Errorf("failed to create policy enforcer: %w", err)
}

ActivePolicy.Name = policyName
ActivePolicy.Name = spec.Type
ActivePolicy.Enforcer = policyEnforcer
return nil
}

func specToPolicyEnforcer(spec configv1alpha1.PolicySpec, policyName string) (policyprovider.PolicyProvider, error) {
policyConfig, err := rawToPolicyConfig(spec.Parameters.Raw, policyName)
func specToPolicyEnforcer(spec configv1alpha1.PolicySpec) (policyprovider.PolicyProvider, error) {
policyConfig, err := rawToPolicyConfig(spec.Parameters.Raw, spec.Type)
if err != nil {
return nil, fmt.Errorf("failed to parse policy config: %w", err)
}
Expand Down
33 changes: 18 additions & 15 deletions pkg/controllers/policy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,30 +145,32 @@ func TestSpecToPolicyEnforcer(t *testing.T) {
expectProvider bool
}{
{
name: "invalid spec",
policyName: policyName1,
spec: configv1alpha1.PolicySpec{},
name: "invalid spec",
policyName: policyName1,
spec: configv1alpha1.PolicySpec{
Type: policyName1,
},
expectErr: true,
expectProvider: false,
},
{
name: "non-supported policy",
policyName: policyName1,
name: "non-supported policy",
spec: configv1alpha1.PolicySpec{
Parameters: runtime.RawExtension{
Raw: []byte("{\"name\": \"policy1\"}"),
},
Type: policyName1,
},
expectErr: true,
expectProvider: false,
},
{
name: "valid spec",
policyName: "configpolicy",
name: "valid spec",
spec: configv1alpha1.PolicySpec{
Parameters: runtime.RawExtension{
Raw: []byte("{\"name\": \"configpolicy\"}"),
},
Type: "configpolicy",
},
expectErr: false,
expectProvider: true,
Expand All @@ -177,7 +179,7 @@ func TestSpecToPolicyEnforcer(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
provider, err := specToPolicyEnforcer(tc.spec, tc.policyName)
provider, err := specToPolicyEnforcer(tc.spec)

if tc.expectErr != (err != nil) {
t.Fatalf("Expected error to be %t, got %t", tc.expectErr, err != nil)
Expand All @@ -197,26 +199,27 @@ func TestPolicyAddOrReplace(t *testing.T) {
expectErr bool
}{
{
name: "invalid spec",
spec: configv1alpha1.PolicySpec{},
policyName: policyName1,
expectErr: true,
name: "invalid spec",
spec: configv1alpha1.PolicySpec{
Type: policyName1,
},
expectErr: true,
},
{
name: "valid spec",
spec: configv1alpha1.PolicySpec{
Parameters: runtime.RawExtension{
Raw: []byte("{\"name\": \"configpolicy\"}"),
},
Type: "configpolicy",
},
policyName: "configpolicy",
expectErr: false,
expectErr: false,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
err := policyAddOrReplace(tc.spec, tc.policyName)
err := policyAddOrReplace(tc.spec)

if tc.expectErr != (err != nil) {
t.Fatalf("Expected error to be %t, got %t", tc.expectErr, err != nil)
Expand Down
Loading