Skip to content

Commit

Permalink
Support templates in OperatorPolicy
Browse files Browse the repository at this point in the history
The Subscription and OperatorGroup fields can now utilize templates. The
resolver uses the DynamicWatcher already used by the controller, so no
additional plumbing was needed for new watches or in order to share a
cache.

Refs:
 - https://issues.redhat.com/browse/ACM-10858

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
  • Loading branch information
JustinKuli committed Apr 26, 2024
1 parent cfe47ca commit 45275dd
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 15 deletions.
57 changes: 51 additions & 6 deletions controllers/operatorpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import (
"reflect"
"regexp"
"slices"
"strconv"
"strings"

operatorv1 "github.com/operator-framework/api/pkg/operators/v1"
operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
templates "github.com/stolostron/go-template-utils/v4/pkg/templates"
depclient "github.com/stolostron/kubernetes-dependency-watches/client"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -316,7 +318,24 @@ func (r *OperatorPolicyReconciler) buildResources(ctx context.Context, policy *p

validationErrors := make([]error, 0)

sub, subErr := buildSubscription(policy, r.DefaultNamespace)
disableTemplates := false

if disableAnnotation, ok := policy.GetAnnotations()["policy.open-cluster-management.io/disable-templates"]; ok {
disableTemplates, _ = strconv.ParseBool(disableAnnotation) // on error, templates will not be disabled
}

var tmplResolver *templates.TemplateResolver

if !disableTemplates {
var err error

tmplResolver, err = templates.NewResolverWithDynamicWatcher(r.DynamicWatcher, templates.Config{})
if err != nil {
validationErrors = append(validationErrors, fmt.Errorf("unable to create template resolver: %w", err))
}
}

sub, subErr := buildSubscription(policy, r.DefaultNamespace, tmplResolver)
if subErr == nil {
err := r.applySubscriptionDefaults(ctx, sub)
if err != nil {
Expand All @@ -339,7 +358,7 @@ func (r *OperatorPolicyReconciler) buildResources(ctx context.Context, policy *p
opGroupNS = sub.Namespace
}

opGroup, ogErr := buildOperatorGroup(policy, opGroupNS)
opGroup, ogErr := buildOperatorGroup(policy, opGroupNS, tmplResolver)
if ogErr != nil {
validationErrors = append(validationErrors, ogErr)
} else {
Expand Down Expand Up @@ -434,13 +453,26 @@ func (r *OperatorPolicyReconciler) applySubscriptionDefaults(
// If an error is returned, it will include details on why the policy spec if invalid and
// why the desired subscription can't be determined.
func buildSubscription(
policy *policyv1beta1.OperatorPolicy, defaultNS string,
policy *policyv1beta1.OperatorPolicy, defaultNS string, tmplResolver *templates.TemplateResolver,
) (*operatorv1alpha1.Subscription, error) {
subscription := new(operatorv1alpha1.Subscription)

rawSub := policy.Spec.Subscription.Raw

if tmplResolver != nil && templates.HasTemplate(rawSub, "", false) {
watcher := opPolIdentifier(policy.Namespace, policy.Name)

resolvedTmpl, err := tmplResolver.ResolveTemplate(rawSub, nil, &templates.ResolveOptions{Watcher: &watcher})
if err != nil {
return nil, fmt.Errorf("could not build subscription: %w", err)
}

rawSub = resolvedTmpl.ResolvedJSON
}

sub := make(map[string]interface{})

err := json.Unmarshal(policy.Spec.Subscription.Raw, &sub)
err := json.Unmarshal(rawSub, &sub)
if err != nil {
return nil, fmt.Errorf("the policy spec.subscription is invalid: %w", err)
}
Expand Down Expand Up @@ -510,7 +542,7 @@ func buildSubscription(
// buildOperatorGroup bootstraps the OperatorGroup spec defined in the operator policy
// with the apiversion and kind in preparation for resource creation
func buildOperatorGroup(
policy *policyv1beta1.OperatorPolicy, namespace string,
policy *policyv1beta1.OperatorPolicy, namespace string, tmplResolver *templates.TemplateResolver,
) (*operatorv1.OperatorGroup, error) {
operatorGroup := new(operatorv1.OperatorGroup)

Expand All @@ -526,9 +558,22 @@ func buildOperatorGroup(
return operatorGroup, nil
}

rawOG := policy.Spec.OperatorGroup.Raw

if tmplResolver != nil && templates.HasTemplate(rawOG, "", false) {
watcher := opPolIdentifier(policy.Namespace, policy.Name)

resolvedTmpl, err := tmplResolver.ResolveTemplate(rawOG, nil, &templates.ResolveOptions{Watcher: &watcher})
if err != nil {
return nil, fmt.Errorf("could not build operator group: %w", err)
}

rawOG = resolvedTmpl.ResolvedJSON
}

opGroup := make(map[string]interface{})

if err := json.Unmarshal(policy.Spec.OperatorGroup.Raw, &opGroup); err != nil {
if err := json.Unmarshal(rawOG, &opGroup); err != nil {
return nil, fmt.Errorf("the policy spec.operatorGroup is invalid: %w", err)
}

Expand Down
6 changes: 3 additions & 3 deletions controllers/operatorpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestBuildSubscription(t *testing.T) {
}

// Check values are correctly bootstrapped to the Subscription
ret, err := buildSubscription(testPolicy, "my-operators")
ret, err := buildSubscription(testPolicy, "my-operators", nil)
assert.Equal(t, err, nil)
assert.Equal(t, ret.GroupVersionKind(), desiredGVK)
assert.Equal(t, ret.ObjectMeta.Name, "my-operator")
Expand Down Expand Up @@ -102,7 +102,7 @@ func TestBuildSubscriptionInvalidNames(t *testing.T) {
}

// Check values are correctly bootstrapped to the Subscription
_, err := buildSubscription(testPolicy, "my-operators")
_, err := buildSubscription(testPolicy, "my-operators", nil)
assert.Equal(t, err.Error(), test.expected)
},
)
Expand Down Expand Up @@ -138,7 +138,7 @@ func TestBuildOperatorGroup(t *testing.T) {
}

// Ensure OperatorGroup values are populated correctly
ret, err := buildOperatorGroup(testPolicy, "my-operators")
ret, err := buildOperatorGroup(testPolicy, "my-operators", nil)
assert.Equal(t, err, nil)
assert.Equal(t, ret.GroupVersionKind(), desiredGVK)
assert.Equal(t, ret.ObjectMeta.GetGenerateName(), "my-operators-")
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
github.com/prometheus/client_golang v1.18.0
github.com/spf13/pflag v1.0.5
github.com/stolostron/go-log-utils v0.1.2
github.com/stolostron/go-template-utils/v4 v4.0.0
github.com/stolostron/go-template-utils/v4 v4.0.2-0.20240426175705-e9547005fb4f
github.com/stolostron/kubernetes-dependency-watches v0.5.2
github.com/stretchr/testify v1.8.4
golang.org/x/mod v0.16.0
Expand Down Expand Up @@ -56,7 +56,7 @@ require (
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/pprof v0.0.0-20240402174815-29b9bb013b0f // indirect
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
github.com/google/uuid v1.3.1 // indirect
github.com/google/uuid v1.4.0 // indirect
github.com/huandu/xstrings v1.4.0 // indirect
github.com/imdario/mergo v1.0.0 // indirect
github.com/josharian/intern v1.0.0 // indirect
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ github.com/google/pprof v0.0.0-20240402174815-29b9bb013b0f/go.mod h1:kf6iHlnVGwg
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.3.1 h1:KjJaJ9iWZ3jOFZIf1Lqf4laDRCasjl0BCmnEGxkdLb4=
github.com/google/uuid v1.3.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.4.0 h1:MtMxsa51/r9yyhkyLsVeVt0B+BGQZzpQiTQ4eHZ8bc4=
github.com/google/uuid v1.4.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/huandu/xstrings v1.3.3/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE=
github.com/huandu/xstrings v1.4.0 h1:D17IlohoQq4UcpqD7fDk80P7l+lwAmlFaBHgOipl2FU=
github.com/huandu/xstrings v1.4.0/go.mod h1:y5/lhBue+AyNmUVz9RLU9xbLR0o4KIIExikq4ovT0aE=
Expand Down Expand Up @@ -142,8 +142,8 @@ github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/stolostron/go-log-utils v0.1.2 h1:7l1aJWvBqU2+DUyimcslT5SJpdygVY/clRDmX5sO29c=
github.com/stolostron/go-log-utils v0.1.2/go.mod h1:8zrB8UJmp1rXhv3Ck9bBl5SpNfKk3SApeElbo96YRtQ=
github.com/stolostron/go-template-utils/v4 v4.0.0 h1:gvSfhXIoymo5Ql9MH/ofTTOtBVkaUBq8HokCGR4xkkc=
github.com/stolostron/go-template-utils/v4 v4.0.0/go.mod h1:svIOPUJpG/ObRn3WwZMBGMEMsBgKH8LVfhsaIArgAAQ=
github.com/stolostron/go-template-utils/v4 v4.0.2-0.20240426175705-e9547005fb4f h1:LKvxEJgNRLetVyMz8HdIRQg+nnojR9OHNL8QpNYiX5I=
github.com/stolostron/go-template-utils/v4 v4.0.2-0.20240426175705-e9547005fb4f/go.mod h1:b93jKpuJtoePu9liTNcp7td+8Hvzk2lROeMqVLC42ks=
github.com/stolostron/kubernetes-dependency-watches v0.5.2 h1:TtctOgPn+TYo1dJ+dr9TLR2rlpy5aFQ/1dzfcYZUDi4=
github.com/stolostron/kubernetes-dependency-watches v0.5.2/go.mod h1:5nwtuleCNR9Mno7SJRz6B2BavvXHbfR4rIwzKLkk9G8=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
Expand Down
102 changes: 102 additions & 0 deletions test/e2e/case38_install_operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2620,4 +2620,106 @@ var _ = Describe("Testing OperatorPolicy", Ordered, func() {
Expect(remBehavior).To(HaveKeyWithValue("customResourceDefinitions", "Keep"))
})
})
Describe("Testing templates in an OperatorPolicy", Ordered, func() {
const (
opPolYAML = "../resources/case38_operator_install/operator-policy-with-templates.yaml"
opPolName = "oppol-with-templates"
configmapYAML = "../resources/case38_operator_install/template-configmap.yaml"
opGroupName = "scoped-operator-group"
subName = "project-quay"
)

BeforeAll(func() {
utils.Kubectl("create", "ns", opPolTestNS)
DeferCleanup(func() {
utils.Kubectl("delete", "ns", opPolTestNS)
})

utils.Kubectl("apply", "-f", configmapYAML, "-n", opPolTestNS)

createObjWithParent(parentPolicyYAML, parentPolicyName,
opPolYAML, opPolTestNS, gvrPolicy, gvrOperatorPolicy)
})

It("Should lookup values for operator policy resources when enforced", func(ctx SpecContext) {
// enforce the policy
utils.Kubectl("patch", "operatorpolicy", opPolName, "-n", opPolTestNS, "--type=json", "-p",
`[{"op": "replace", "path": "/spec/remediationAction", "value": "enforce"}]`)

check(
opPolName,
false,
[]policyv1.RelatedObject{{
Object: policyv1.ObjectResource{
Kind: "OperatorGroup",
APIVersion: "operators.coreos.com/v1",
Metadata: policyv1.ObjectMetadata{
Name: opGroupName,
Namespace: opPolTestNS,
},
},
Compliant: "Compliant",
Reason: "Resource found as expected",
}},
metav1.Condition{
Type: "OperatorGroupCompliant",
Status: metav1.ConditionTrue,
Reason: "OperatorGroupMatches",
Message: "the OperatorGroup matches what is required by the policy",
},
"the OperatorGroup required by the policy was created",
)

By("Verifying the targetNamespaces in the OperatorGroup")
og, err := clientManagedDynamic.Resource(gvrOperatorGroup).Namespace(opPolTestNS).
Get(ctx, opGroupName, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(og).NotTo(BeNil())

targetNamespaces, found, err := unstructured.NestedStringSlice(og.Object, "spec", "targetNamespaces")
Expect(err).NotTo(HaveOccurred())
Expect(found).To(BeTrue())
Expect(targetNamespaces).To(HaveExactElements("foo", "bar", opPolTestNS))

By("Verifying the Subscription channel")
sub, err := clientManagedDynamic.Resource(gvrSubscription).Namespace(opPolTestNS).
Get(ctx, subName, metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred())
Expect(sub).NotTo(BeNil())

channel, found, err := unstructured.NestedString(sub.Object, "spec", "channel")
Expect(err).NotTo(HaveOccurred())
Expect(found).To(BeTrue())
Expect(channel).To(Equal("fakechannel"))
})

It("Should update the subscription after the configmap is updated", func(ctx SpecContext) {
utils.Kubectl("patch", "configmap", "op-config", "-n", opPolTestNS, "--type=json", "-p",
`[{"op": "replace", "path": "/data/channel", "value": "stable-3.8"}]`)

check(
opPolName,
false,
[]policyv1.RelatedObject{{
Object: policyv1.ObjectResource{
Kind: "Subscription",
APIVersion: "operators.coreos.com/v1alpha1",
Metadata: policyv1.ObjectMetadata{
Name: subName,
Namespace: opPolTestNS,
},
},
Compliant: "Compliant",
Reason: "Resource found as expected",
}},
metav1.Condition{
Type: "SubscriptionCompliant",
Status: metav1.ConditionTrue,
Reason: "SubscriptionMatches",
Message: "the Subscription matches what is required by the policy",
},
"the Subscription was updated to match the policy",
)
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
apiVersion: policy.open-cluster-management.io/v1beta1
kind: OperatorPolicy
metadata:
name: oppol-with-templates
annotations:
policy.open-cluster-management.io/parent-policy-compliance-db-id: "124"
policy.open-cluster-management.io/policy-compliance-db-id: "64"
ownerReferences:
- apiVersion: policy.open-cluster-management.io/v1
kind: Policy
name: parent-policy
uid: 12345678-90ab-cdef-1234-567890abcdef # must be replaced before creation
spec:
remediationAction: inform
severity: medium
complianceType: musthave
operatorGroup: # optional
name: scoped-operator-group
namespace: operator-policy-testns
targetNamespaces: '{{ (fromConfigMap "operator-policy-testns" "op-config" "namespaces") | toLiteral }}'
subscription:
channel: '{{ (lookup "v1" "ConfigMap" "operator-policy-testns" "op-config").data.channel }}'
name: project-quay
namespace: operator-policy-testns
installPlanApproval: Automatic
source: operatorhubio-catalog
sourceNamespace: olm
startingCSV: quay-operator.v3.8.1
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: v1
kind: ConfigMap
metadata:
name: op-config
data:
channel: "fakechannel"
namespaces: '["foo", "bar", "operator-policy-testns"]'

0 comments on commit 45275dd

Please sign in to comment.