Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
  • Loading branch information
JustinKuli committed Apr 26, 2024
1 parent 0947bb7 commit 8cc4bf1
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 16 deletions.
65 changes: 59 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,30 @@ 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)
opts := templates.ResolveOptions{
Watcher: &watcher,
ErrorOnMissingKey: true,
}

resolvedTmpl, err := tmplResolver.ResolveTemplate(rawSub, nil, &opts)
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 +546,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 +562,26 @@ func buildOperatorGroup(
return operatorGroup, nil
}

rawOG := policy.Spec.OperatorGroup.Raw

if tmplResolver != nil && templates.HasTemplate(rawOG, "", false) {
watcher := opPolIdentifier(policy.Namespace, policy.Name)
opts := templates.ResolveOptions{
Watcher: &watcher,
ErrorOnMissingKey: true,
}

resolvedTmpl, err := tmplResolver.ResolveTemplate(rawOG, nil, &opts)
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
18 changes: 16 additions & 2 deletions controllers/operatorpolicy_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package controllers

import (
"context"
"errors"
"fmt"
"sort"
"strings"
"time"

operatorv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
"github.com/stolostron/go-template-utils/v4/pkg/templates"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -178,7 +180,14 @@ func calculateComplianceCondition(policy *policyv1beta1.OperatorPolicy) metav1.C
messages = append(messages, "the validity of the policy is unknown")
foundNonCompliant = true
} else {
messages = append(messages, cond.Message)
// This message can be very long: shorten it for the Compliance condition
// (the full message will still be in the status, in the ValidPolicySpec condition)
if cond.Reason == "MissingKeyInTemplate" {
lastPartToKeep := "missing key while resolving template"
messages = append(messages, strings.Split(cond.Message, lastPartToKeep)[0]+lastPartToKeep)
} else {
messages = append(messages, cond.Message)
}

if cond.Status != metav1.ConditionTrue {
foundNonCompliant = true
Expand Down Expand Up @@ -569,16 +578,21 @@ func validationCond(validationErrors []error) metav1.Condition {
}
}

reason := "InvalidPolicySpec"
msgs := make([]string, len(validationErrors))

for i, err := range validationErrors {
msgs[i] = err.Error()

if errors.Is(err, templates.ErrMissingKey) {
reason = "MissingKeyInTemplate"
}
}

return metav1.Condition{
Type: validPolicyConditionType,
Status: metav1.ConditionFalse,
Reason: "InvalidPolicySpec",
Reason: reason,
Message: strings.Join(msgs, ", "),
}
}
Expand Down
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
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 Expand Up @@ -110,3 +110,5 @@ replace (
github.com/imdario/mergo => github.com/imdario/mergo v0.3.16 // Replaced so that 'go get -u' works. Remove/bump when upgrading.
k8s.io/kubectl => k8s.io/kubectl v0.28.0 // Replaced so that 'go get -u' works. Remove/bump when upgrading.
)

replace github.com/stolostron/go-template-utils/v4 => github.com/JustinKuli/go-template-utils/v4 v4.1.0-alpha2 // For pre-release testing
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/JustinKuli/go-template-utils/v4 v4.1.0-alpha2 h1:epSmYsBpvtH+95aXETIx0/zxsWZS01RbG7D0cJ3lazQ=
github.com/JustinKuli/go-template-utils/v4 v4.1.0-alpha2/go.mod h1:b93jKpuJtoePu9liTNcp7td+8Hvzk2lROeMqVLC42ks=
github.com/Masterminds/goutils v1.1.1 h1:5nUrii3FMTL5diU80unEVvNevw1nH4+ZV4DSLVJLSYI=
github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU=
github.com/Masterminds/semver/v3 v3.2.0/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ=
Expand Down Expand Up @@ -69,8 +71,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 +144,6 @@ 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/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
136 changes: 136 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,140 @@ 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)
})

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

It("Should have a validation error when the configmap does not exist", func(ctx SpecContext) {
check(
opPolName,
true,
[]policyv1.RelatedObject{},
metav1.Condition{
Type: "ValidPolicySpec",
Status: metav1.ConditionFalse,
Reason: "MissingKeyInTemplate",
// This condition has the full error, but this test only checks for a snippet of it
Message: `could not build subscription: missing key while resolving template {"channel`,
},
// The compliance event should not give the full details about the template problem
`could not build subscription: missing key while resolving template, `+
`the status of the OperatorGroup could not be determined`,
)
})

It("Should remove the validation error when the configmap is created", func(ctx SpecContext) {
utils.Kubectl("apply", "-f", configmapYAML, "-n", opPolTestNS)
check(
opPolName,
true,
[]policyv1.RelatedObject{},
metav1.Condition{
Type: "ValidPolicySpec",
Status: metav1.ConditionTrue,
Reason: "PolicyValidated",
Message: "the policy spec is valid",
},
`the policy spec is valid`,
)
})

It("Should lookup values and inject them into the 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"},`+
`{"op": "add", "path": "/spec/operatorGroup/targetNamespaces", "value":`+
`"{{ (fromConfigMap \"operator-policy-testns\" \"op-config\" \"namespaces\") | toLiteral}}"}]`)

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",
)
})
})
})
Loading

0 comments on commit 8cc4bf1

Please sign in to comment.