Skip to content

Commit

Permalink
Add rule type change detection in profile parse
Browse files Browse the repository at this point in the history
This PR checks if a rule is the checktype of a rule is being changed, and it adds an annotation if it does.
  • Loading branch information
Vincent056 committed Sep 12, 2023
1 parent 0ff9b99 commit dcd2c5d
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 0 deletions.
6 changes: 6 additions & 0 deletions pkg/apis/compliance/v1alpha1/tailoredprofile_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ import (

// FIXME: move name/rationale to a common struct with an interface?

// OutdatedReferenceValidationDisable a label is used to disable validation of outdated references
const OutdatedReferenceValidationDisable = "compliance.openshift.io/outdated-reference-validation-disable"

// RuleLastCheckTypeChangedAnnotationKey is the annotation key used to indicate that the rule check type has changed, store its previous check type
const RuleLastCheckTypeChangedAnnotationKey = "compliance.openshift.io/rule-last-check-type"

// RuleReferenceSpec specifies a rule to be selected/deselected, as well as the reason why
type RuleReferenceSpec struct {
// Name of the rule that's being referenced
Expand Down
8 changes: 8 additions & 0 deletions pkg/profileparser/profileparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ func ParseBundle(contentDom *xmlquery.Node, pb *cmpv1alpha1.ProfileBundle, pcfg
}

foundRule.Annotations = updatedRule.Annotations
// if the check type has changed, add an annotation to the rule
// to indicate that the rule needs to be checked in TailoredProfile validation
if foundRule.CheckType != updatedRule.CheckType {
log.Info("Rule check type has changed", "rule", foundRule.Name, "oldCheckType", foundRule.CheckType, "newCheckType", updatedRule.CheckType)
foundRule.Annotations[cmpv1alpha1.RuleLastCheckTypeChangedAnnotationKey] = foundRule.CheckType
}
foundRule.RulePayload = *updatedRule.RulePayload.DeepCopy()
return pcfg.Client.Update(context.TODO(), foundRule)
})
Expand Down Expand Up @@ -207,6 +213,7 @@ func createOrUpdate(cli runtimeclient.Client, kind string, key types.NamespacedN
updateTo := obj.DeepCopyObject()
err := cli.Get(context.TODO(), key, found)
if errors.IsNotFound(err) {
log.Info("Object not found, creating", "kind", kind, "key", key)
err := cli.Create(context.TODO(), obj)
if err != nil {
log.Error(err, "Failed to create object", "kind", kind, "key", key)
Expand Down Expand Up @@ -594,6 +601,7 @@ func ParseRulesAndDo(contentDom *xmlquery.Node, stdParser *referenceParser, pb *
}

rawFixReader := strings.NewReader(fixNodeObj.InnerText())
log.Info("Raw fix", "rawFix", fixNodeObj.InnerText())
fixKubeObjs, err := utils.ReadObjectsFromYAML(rawFixReader)
if err != nil {
log.Info("Couldn't parse Kubernetes object from fix")
Expand Down
16 changes: 16 additions & 0 deletions tests/e2e/framework/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,22 @@ func (f *Framework) AssertMustHaveParsedProfiles(pbName, productType, productNam
return nil
}

// AssertRuleCheckTypeChangedAnnotationKey asserts that the rule check type changed annotation key exists
func (f *Framework) AssertRuleCheckTypeChangedAnnotationKey(namespace, ruleName, lastCheckType string) error {
var r compv1alpha1.Rule
key := types.NamespacedName{Namespace: namespace, Name: ruleName}
if err := f.Client.Get(context.Background(), key, &r); err != nil {
return err
}
if r.Annotations == nil {
return fmt.Errorf("expected annotations to be not nil")
}
if r.Annotations[compv1alpha1.RuleLastCheckTypeChangedAnnotationKey] != lastCheckType {
return fmt.Errorf("expected %s to be %s, got %s instead", compv1alpha1.RuleLastCheckTypeChangedAnnotationKey, lastCheckType, r.Annotations[compv1alpha1.RuleLastCheckTypeChangedAnnotationKey])
}
return nil
}

func (f *Framework) DoesRuleExist(namespace, ruleName string) (error, bool) {
err, found := f.DoesObjectExist("Rule", namespace, ruleName)
if err != nil {
Expand Down
72 changes: 72 additions & 0 deletions tests/e2e/parallel/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,78 @@ func TestProfileModification(t *testing.T) {
}
}

func TestRuleCheckType(t *testing.T) {
t.Parallel()
f := framework.Global
const (
changeTypeRule = "kubelet-anonymous-auth"
moderateProfileName = "moderate"
)
var (
baselineImage = fmt.Sprintf("%s:%s", brokenContentImagePath, "kubelet_default")
modifiedImage = fmt.Sprintf("%s:%s", brokenContentImagePath, "new_kubeletconfig")
)

prefixName := func(profName, ruleBaseName string) string { return profName + "-" + ruleBaseName }

pbName := framework.GetObjNameFromTest(t)
origPb := &compv1alpha1.ProfileBundle{
ObjectMeta: metav1.ObjectMeta{
Name: pbName,
Namespace: f.OperatorNamespace,
},
Spec: compv1alpha1.ProfileBundleSpec{
ContentImage: baselineImage,
ContentFile: framework.OcpContentFile,
},
}
// Pass nil in as the cleanupOptions since so we don't invoke all the
// cleanup function code in Create. Use defer to cleanup the
// ProfileBundle at the end of the test, instead of at the end of the
// suite.
if err := f.Client.Create(context.TODO(), origPb, nil); err != nil {
t.Fatalf("failed to create ProfileBundle: %s", err)
}
// This should get cleaned up at the end of the test
defer f.Client.Delete(context.TODO(), origPb)

if err := f.WaitForProfileBundleStatus(pbName, compv1alpha1.DataStreamValid); err != nil {
t.Fatalf("failed waiting for the ProfileBundle to become available: %s", err)
}

// Check that the rule exists in the original profile
changeTypeRuleName := prefixName(pbName, changeTypeRule)
err, found := f.DoesRuleExist(origPb.Namespace, changeTypeRuleName)
if err != nil {
t.Fatal(err)
} else if found != true {
t.Fatalf("expected rule %s to exist in namespace %s", changeTypeRuleName, origPb.Namespace)
}

// update the image with a new hash
modPb := origPb.DeepCopy()
if err := f.Client.Get(context.TODO(), types.NamespacedName{Namespace: modPb.Namespace, Name: modPb.Name}, modPb); err != nil {
t.Fatalf("failed to get ProfileBundle %s", modPb.Name)
}

modPb.Spec.ContentImage = modifiedImage
if err := f.Client.Update(context.TODO(), modPb); err != nil {
t.Fatalf("failed to update ProfileBundle %s: %s", modPb.Name, err)
}

// Wait for the update to happen, the PB will flip first to pending, then to valid
if err := f.WaitForProfileBundleStatus(pbName, compv1alpha1.DataStreamValid); err != nil {
t.Fatalf("failed to parse ProfileBundle %s: %s", pbName, err)
}

if err := f.AssertProfileBundleMustHaveParsedRules(pbName); err != nil {
t.Fatal(err)
}
if err := f.AssertRuleCheckTypeChangedAnnotationKey(f.OperatorNamespace, changeTypeRuleName, "Platform"); err != nil {
t.Fatal(err)
}
}

func TestProfileISTagUpdate(t *testing.T) {
t.Parallel()
f := framework.Global
Expand Down

0 comments on commit dcd2c5d

Please sign in to comment.