Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1560,6 +1560,7 @@ spec:
- watch
- list
- update
- patch
- apiGroups:
- compliance.openshift.io
resources:
Expand Down
1 change: 1 addition & 0 deletions config/rbac/profileparser_role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ rules:
- watch
- list
- update
- patch
- apiGroups:
- compliance.openshift.io
resources:
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/compliance/v1alpha1/profilebundle_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ const ProfileImageDigestAnnotation = "compliance.openshift.io/image-digest"
// ProfileStatusAnnotation is the parsed out status from the data stream
const ProfileStatusAnnotation = "compliance.openshift.io/profile-status"

// XCCDFGroupsAnnotation stores a comma-separated list of all XCCDF Group IDs
// found in the datastream. Used for re-enabling groups in TailoredProfiles.
const XCCDFGroupsAnnotation = "compliance.openshift.io/xccdf-groups"

// DataStreamStatusType is the type for the data stream status
type DataStreamStatusType string

Expand Down
47 changes: 47 additions & 0 deletions pkg/profileparser/profileparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ func GetPrefixedName(pbName, objName string) string {
}

func ParseBundle(contentDom *xmlquery.Node, pb *cmpv1alpha1.ProfileBundle, pcfg *ParserConfig) error {
// Extract all XCCDF Group IDs from the datastream and store in ProfileBundle annotation
if err := extractAndStoreXCCDFGroups(contentDom, pb, pcfg); err != nil {
log.Error(err, "Failed to extract XCCDF groups")
// Don't fail the whole parse if group extraction fails
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we surface this error in case the user needs to use those rules, and fail the bundle as an error state? or if we do not want to fail entire profilebundle we could in TailoredProfileToXML, if the annotation is missing AND the TP extends a profile, enable groups based on a query of the live datastream or a sentinel "enable all groups" wildcard.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have have the ProfileBundle in error state.

Another alternative is to have the tailoring controller extract the groups at creation time, rather than Profile Bundle creation.
That would avoid us having the annotation in PB.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I prefer the current approach, with an annotation in the ProfileBundle.

}

// One go routine per type
errChan := make(chan error)
done := make(chan string)
Expand Down Expand Up @@ -950,3 +956,44 @@ func appendKeyWithSep(annotations map[string]string, key, item, sep string) {
}
annotations[key] = strings.Join(append(curList, item), sep)
}

// extractAndStoreXCCDFGroups extracts all XCCDF Group IDs from the datastream
// and stores them as a comma-separated list in the ProfileBundle annotation.
// This allows TailoredProfiles to re-enable all groups when extending a parent profile.
func extractAndStoreXCCDFGroups(contentDom *xmlquery.Node, pb *cmpv1alpha1.ProfileBundle, pcfg *ParserConfig) error {
// Find all Group elements in the datastream
groupNodes := xmlquery.Find(contentDom, "//xccdf-1.2:Group")
if len(groupNodes) == 0 {
log.Info("No XCCDF groups found in datastream")
return nil
}

groupIDs := make([]string, 0, len(groupNodes))
for _, groupNode := range groupNodes {
id := groupNode.SelectAttr("id")
if id != "" {
groupIDs = append(groupIDs, id)
}
}

if len(groupIDs) == 0 {
return nil
}

// Store as comma-separated list in ProfileBundle annotation
patch := runtimeclient.MergeFrom(pb.DeepCopy())
annotations := pb.GetAnnotations()
if annotations == nil {
annotations = make(map[string]string)
}
annotations[cmpv1alpha1.XCCDFGroupsAnnotation] = strings.Join(groupIDs, ",")
pb.SetAnnotations(annotations)

// Patch the ProfileBundle with the new annotation
if err := pcfg.Client.Patch(context.TODO(), pb, patch); err != nil {
return fmt.Errorf("failed to patch ProfileBundle with XCCDF groups: %w", err)
}

log.Info("Extracted XCCDF groups", "count", len(groupIDs), "profileBundle", pb.Name)
return nil
}
60 changes: 60 additions & 0 deletions pkg/profileparser/profileparser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package profileparser
import (
"context"
"os"
"strings"

compapis "github.com/ComplianceAsCode/compliance-operator/pkg/apis"
cmpv1alpha1 "github.com/ComplianceAsCode/compliance-operator/pkg/apis/compliance/v1alpha1"
"github.com/antchfx/xmlquery"
"github.com/go-logr/zapr"
Expand All @@ -19,6 +21,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/apiserver/pkg/storage/names"
runtimeclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
)

// FIXME: code duplication
Expand Down Expand Up @@ -712,3 +715,60 @@ var _ = Describe("Testing CPE string parsing in isolation", func() {
})
})
})

var _ = Describe("Testing extractAndStoreXCCDFGroups", func() {
var (
testPb *cmpv1alpha1.ProfileBundle
testClient runtimeclient.Client
testPcfg *ParserConfig
)

BeforeEach(func() {
testScheme := k8sruntime.NewScheme()
err := compapis.AddToScheme(testScheme)
Expect(err).To(BeNil())

testPb = &cmpv1alpha1.ProfileBundle{
ObjectMeta: metav1.ObjectMeta{
Name: "test-pb",
Namespace: testNamespace,
},
}

testClient = fake.NewClientBuilder().WithScheme(testScheme).WithObjects(testPb).Build()
testPcfg = &ParserConfig{Client: testClient}
})

It("Extracts and stores group IDs from datastream", func() {
xmlContent := `<?xml version="1.0" encoding="UTF-8"?>
<root xmlns:xccdf-1.2="http://checklists.nist.gov/xccdf/1.2">
<xccdf-1.2:Group id="group1"/>
<xccdf-1.2:Group id="group2"/>
</root>`
contentDom, err := xmlquery.Parse(strings.NewReader(xmlContent))
Expect(err).To(BeNil())

err = extractAndStoreXCCDFGroups(contentDom, testPb, testPcfg)
Expect(err).To(BeNil())

updated := &cmpv1alpha1.ProfileBundle{}
err = testClient.Get(context.TODO(), types.NamespacedName{Name: testPb.Name, Namespace: testPb.Namespace}, updated)
Expect(err).To(BeNil())

annotations := updated.GetAnnotations()
Expect(annotations).To(HaveKey(cmpv1alpha1.XCCDFGroupsAnnotation))
Expect(annotations[cmpv1alpha1.XCCDFGroupsAnnotation]).To(Equal("group1,group2"))
})

It("Returns nil when no groups are found", func() {
xmlContent := `<?xml version="1.0" encoding="UTF-8"?>
<root xmlns:xccdf-1.2="http://checklists.nist.gov/xccdf/1.2">
<xccdf-1.2:Profile id="test-profile"/>
</root>`
contentDom, err := xmlquery.Parse(strings.NewReader(xmlContent))
Expect(err).To(BeNil())

err = extractAndStoreXCCDFGroups(contentDom, testPb, testPcfg)
Expect(err).To(BeNil())
})
})
41 changes: 39 additions & 2 deletions pkg/xccdf/tailoring.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ import (
"github.com/google/uuid"

cmpv1alpha1 "github.com/ComplianceAsCode/compliance-operator/pkg/apis/compliance/v1alpha1"
logf "sigs.k8s.io/controller-runtime/pkg/log"
)

var log = logf.Log.WithName("xccdf")

const (
// XMLHeader is the header for the XML doc
XMLHeader string = `<?xml version="1.0" encoding="UTF-8"?>`
Expand Down Expand Up @@ -146,8 +149,19 @@ func getSelectElementFromCRRule(rule *cmpv1alpha1.Rule, enable bool) SelectEleme
}
}

func getSelections(tp *cmpv1alpha1.TailoredProfile, rules map[string]*cmpv1alpha1.Rule) []SelectElement {
func getSelections(tp *cmpv1alpha1.TailoredProfile, rules map[string]*cmpv1alpha1.Rule, groupIDs []string) []SelectElement {
selections := []SelectElement{}

// When extending a profile, enable all XCCDF groups first
// This allows individual rules within deselected groups to be enabled
// Groups are enabled before rules so OpenSCAP processes them in the correct order
for _, groupID := range groupIDs {
selections = append(selections, SelectElement{
IDRef: groupID,
Selected: true,
})
}

for _, selection := range tp.Spec.EnableRules {
rule := rules[selection.Name]
selections = append(selections, getSelectElementFromCRRule(rule, true))
Expand Down Expand Up @@ -200,6 +214,29 @@ func getValuesFromVariables(variables []*cmpv1alpha1.Variable) []SetValueElement

// TailoredProfileToXML gets an XML string from a TailoredProfile and the corresponding Profile
func TailoredProfileToXML(tp *cmpv1alpha1.TailoredProfile, p *cmpv1alpha1.Profile, pb *cmpv1alpha1.ProfileBundle, rules map[string]*cmpv1alpha1.Rule, variables []*cmpv1alpha1.Variable) (string, error) {
if pb == nil {
return "", fmt.Errorf("ProfileBundle cannot be nil")
}

// Extract group IDs from ProfileBundle annotation if this TP extends a profile
var groupIDs []string
if p != nil {
Comment thread
yuumasato marked this conversation as resolved.
if pb.Annotations != nil {
if groupsStr, ok := pb.Annotations[cmpv1alpha1.XCCDFGroupsAnnotation]; ok && groupsStr != "" {
groupIDs = strings.Split(groupsStr, ",")
} else {
log.Info("ProfileBundle is missing XCCDF groups annotation - groups will not be enabled in tailoring",
"profileBundle", pb.Name,
"tailoredProfile", tp.Name,
"annotation", cmpv1alpha1.XCCDFGroupsAnnotation)
}
} else {
log.Info("ProfileBundle has no annotations - groups will not be enabled in tailoring",
"profileBundle", pb.Name,
"tailoredProfile", tp.Name)
}
}

tailoring := TailoringElement{
XMLNamespaceURI: XCCDFURI,
ID: getTailoringID(tp),
Expand All @@ -215,7 +252,7 @@ func TailoredProfileToXML(tp *cmpv1alpha1.TailoredProfile, p *cmpv1alpha1.Profil
},
Profile: ProfileElement{
ID: GetXCCDFProfileID(tp),
Selections: getSelections(tp, rules),
Selections: getSelections(tp, rules, groupIDs),
Values: getValuesFromVariables(variables),
},
}
Expand Down
44 changes: 44 additions & 0 deletions tests/e2e/parallel/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,50 @@ func TestProfileVersion(t *testing.T) {
}
}

func TestProfileBundleXCCDFGroupsAnnotation(t *testing.T) {
t.Parallel()
f := framework.Global

pbName := framework.GetObjNameFromTest(t)
pb, err := f.CreateProfileBundle(pbName, contentImagePath, framework.RhcosContentFile)
if err != nil {
t.Fatalf("failed to create ProfileBundle: %s", err)
}
defer f.Client.Delete(context.TODO(), pb)

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

// Get the updated ProfileBundle to check annotations
updatedPb := &compv1alpha1.ProfileBundle{}
if err := f.Client.Get(context.TODO(), types.NamespacedName{Name: pbName, Namespace: f.OperatorNamespace}, updatedPb); err != nil {
t.Fatalf("failed to get ProfileBundle %s: %s", pbName, err)
}

annotations := updatedPb.GetAnnotations()
if annotations == nil {
t.Fatalf("ProfileBundle %s has no annotations", pbName)
}

groupsAnnotation, exists := annotations[compv1alpha1.XCCDFGroupsAnnotation]
if !exists {
t.Fatalf("ProfileBundle %s is missing the %s annotation", pbName, compv1alpha1.XCCDFGroupsAnnotation)
}

if groupsAnnotation == "" {
t.Fatalf("ProfileBundle %s has empty %s annotation", pbName, compv1alpha1.XCCDFGroupsAnnotation)
}

// Verify it's a comma-separated list with at least one group
groups := strings.Split(groupsAnnotation, ",")
if len(groups) == 0 {
t.Fatalf("ProfileBundle %s has no groups in %s annotation", pbName, compv1alpha1.XCCDFGroupsAnnotation)
}

t.Logf("ProfileBundle %s has %d XCCDF groups", pbName, len(groups))
}

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