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

OCPBUGS-6827: ssb: Presume Platform scan if no scan type is given with annotations #195

Merged
merged 1 commit into from
Feb 9, 2023
Merged
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ Versioning](https://semver.org/spec/v2.0.0.html).
a buggy comparison that did not take into account trivial differences
in remediation metadata (tracked with [OCPBUGS-6710](https://issues.redhat.com/browse/OCPBUGS-6710))

- Fixed a [regression](https://issues.redhat.com/browse/OCPBUGS-6827) where
attempting to create a `ScanSettingBinding` that was using a `TailoredProfile`
that was in turn using a non-default `MachineConfigPool` would mark
the `ScanSettingBinding` as failed. Note that this bug was only affecting
setups where the TailoredProfile wasn't annotated with
`compliance.openshift.io/product-type`.

### Internal Changes

-
Expand Down
55 changes: 51 additions & 4 deletions pkg/controller/scansettingbinding/scansettingbinding_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -422,14 +422,15 @@ func (r *ReconcileScanSettingBinding) verifyRoleInScanSettingBinding(
prfRef := &binding.Profiles[i]

key := types.NamespacedName{Namespace: binding.Namespace, Name: prfRef.Name}
tp, err := getUnstructured(r, binding, key, prfRef.Kind, prfRef.APIGroup, logger)
prfObj, err := getUnstructured(r, binding, key, prfRef.Kind, prfRef.APIGroup, logger)
if err != nil {
return fmt.Errorf("error getting TailoredProfile %s: %w", prfRef.Name, err)
return fmt.Errorf("error getting Profile %s: %w", prfRef.Name, err)
}

profileType, err := getScanType(tp.GetAnnotations())
profileType, err := getTpScanType(r, binding, prfRef.APIGroup, prfObj, logger)
if err != nil {
return fmt.Errorf("error getting profile type %s: %w", prfRef.Name, err)
logger.Info("error getting scan type, assuming Platform", "profile", prfRef.Name, "error", err)
profileType = compliancev1alpha1.ScanTypePlatform
jhrozek marked this conversation as resolved.
Show resolved Hide resolved
rhmdnd marked this conversation as resolved.
Show resolved Hide resolved
}

if profileType == compliancev1alpha1.ScanTypeNode {
Expand Down Expand Up @@ -681,6 +682,52 @@ func getScanType(annotations map[string]string) (compliancev1alpha1.ComplianceSc
return compliancev1alpha1.ScanTypePlatform, nil
}

func getTpScanType(
r *ReconcileScanSettingBinding,
binding *compliancev1alpha1.ScanSettingBinding,
apiGroup string,
prfObj *unstructured.Unstructured,
logger logr.Logger,
) (compliancev1alpha1.ComplianceScanType, error) {
profileType, err := getScanType(prfObj.GetAnnotations())
if err == nil {
return profileType, nil
} else if prfObj.GetKind() != "TailoredProfile" {
// profiles are pretty much always annotated. If not, let's just assume it's a platform scan. If not,
// the scan would just fail later..
logger.Info("error getting profile scan type, assuming Platform", "profile", prfObj.GetName(), "error", err)
return compliancev1alpha1.ScanTypePlatform, nil
}

logger.Info("TailoredProfile had no annotation, trying the parent profile", "tailoredProfile", prfObj.GetName())
val, found, nsErr := unstructured.NestedString(prfObj.Object, "spec", "extends")
if nsErr != nil {
logger.Error(nsErr, "Fetching state of tailored profile",
"TailoredProfile", prfObj.GetName())
return compliancev1alpha1.ScanTypePlatform, nsErr
} else if !found {
// if there is no extends, then this is a custom-created profile. Custom profiles would indicate that they
// are node profiles either by using the -node suffix or by being properly annotated. Otherwise, even
// the plain scan profile would assume platform, so let's do the same here.
logger.Info("No extends field found in tailored profile", "TailoredProfile", prfObj.GetName())
return compliancev1alpha1.ScanTypePlatform, nil
}

key := types.NamespacedName{Namespace: prfObj.GetNamespace(), Name: val}
extendsObj, err := getUnstructured(r, binding, key, "Profile", apiGroup, logger)
if err != nil {
return compliancev1alpha1.ScanTypePlatform, fmt.Errorf("error getting Profile %s: %w", val, err)
}

extendsType, err := getScanType(extendsObj.GetAnnotations())
if err != nil {
return extendsType, common.NewNonRetriableCtrlError("error getting scan type for profile %s: %w", extendsObj.GetName(), err)
}

logger.Info("Got scan type from parent profile", "tailoredProfile", prfObj.GetName(), "profile", extendsObj.GetName(), "type", extendsType)
return extendsType, nil
}

func resolveProfileReference(r *ReconcileScanSettingBinding, instance *compliancev1alpha1.ScanSettingBinding, profile *unstructured.Unstructured, logger logr.Logger) (*profileReference, error) {
var profReference profileReference
var err error
Expand Down
70 changes: 70 additions & 0 deletions tests/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2331,6 +2331,76 @@ func TestE2E(t *testing.T) {
return nil
},
},
testExecution{
Name: "TestScanSettingBindingTailoringAndNonDefaultRole",
IsParallel: true,
TestFn: func(t *testing.T, f *framework.Framework, ctx *framework.Context, mcTctx *mcTestCtx, namespace string) error {
mcTctx.ensureE2EPool()

tpName := "non-default-role-tp"
scanSettingBindingName := "non-default-role-ssb"

tp := &compv1alpha1.TailoredProfile{
ObjectMeta: metav1.ObjectMeta{
Name: tpName,
Namespace: namespace,
},
Spec: compv1alpha1.TailoredProfileSpec{
Title: "TestCisForE2EPool",
Description: "TestCisForE2EPool",
Extends: "ocp4-cis",
SetValues: []compv1alpha1.VariableValueSpec{
{
Name: "ocp4-var-role-master",
Rationale: "targets the e2e pool",
Value: "e2e",
},
{
Name: "ocp4-var-role-worker",
Rationale: "targets the e2e pool",
Value: "e2e",
},
},
},
}

createTPErr := f.Client.Create(goctx.TODO(), tp, getCleanupOpts(ctx))
if createTPErr != nil {
return createTPErr
}

scanSettingBinding := compv1alpha1.ScanSettingBinding{
ObjectMeta: metav1.ObjectMeta{
Name: scanSettingBindingName,
Namespace: namespace,
},
Profiles: []compv1alpha1.NamedObjectReference{
{
Name: tpName,
Kind: "TailoredProfile",
APIGroup: "compliance.openshift.io/v1alpha1",
},
},
SettingsRef: &compv1alpha1.NamedObjectReference{
Name: "e2e-default",
Kind: "ScanSetting",
APIGroup: "compliance.openshift.io/v1alpha1",
},
}

err := f.Client.Create(goctx.TODO(), &scanSettingBinding, getCleanupOpts(ctx))
if err != nil {
return err
}

// it's enough to check that the scan finishes
if err := waitForSuiteScansStatus(t, f, namespace, scanSettingBindingName, compv1alpha1.PhaseDone, compv1alpha1.ResultNonCompliant); err != nil {
return err
}

return nil
},
},
testExecution{
Name: "TestScanSettingBindingUsesDefaultScanSetting",
IsParallel: true,
Expand Down