Add more cases to TestLintBlueprint covering the warnings

Author: mgold1234

internal/v1/handler_blueprints_test.go

@@ -1497,24 +1497,31 @@ func TestLintBlueprint(t *testing.T) {
 	}))
 
 	cases := []struct {
-		blueprint  v1.BlueprintBody
-		lintErrors []v1.BlueprintLintItem
+		name         string
+		blueprint    v1.BlueprintBody
+		snapshot     *json.RawMessage
+		lintErrors   []v1.BlueprintLintItem
+		lintWarnings []v1.BlueprintLintItem
 	}{
 		{
+			name: "missing packages and services",
 			blueprint: v1.BlueprintBody{
 				Distribution: "rhel-8",
 				Customizations: v1.Customizations{
 					Openscap: &oscap,
 				},
 			},
+			snapshot: nil,
 			lintErrors: []v1.BlueprintLintItem{
 				{Name: "Compliance", Description: "package required-by-compliance required by policy is not present"},
 				{Name: "Compliance", Description: "service enabled-required-by-compliance required as enabled by policy is not present"},
 				{Name: "Compliance", Description: "service masked-required-by-compliance required as masked by policy is not present"},
 				{Name: "Compliance", Description: "FIPS required 'true' by policy but not set"},
 			},
+			lintWarnings: []v1.BlueprintLintItem{},
 		},
 		{
+			name: "all requirements satisfied",
 			blueprint: v1.BlueprintBody{
 				Distribution: "rhel-8",
 				Customizations: v1.Customizations{
@@ -1535,22 +1542,28 @@ func TestLintBlueprint(t *testing.T) {
 					},
 				},
 			},
-			lintErrors: []v1.BlueprintLintItem{},
+			snapshot:     nil,
+			lintErrors:   []v1.BlueprintLintItem{},
+			lintWarnings: []v1.BlueprintLintItem{},
 		},
 		{
+			name: "missing filesystems and kernel params",
 			blueprint: v1.BlueprintBody{
 				Distribution: "rhel-8",
 				Customizations: v1.Customizations{
 					Openscap: &oscap2,
 				},
 			},
+			snapshot: nil,
 			lintErrors: []v1.BlueprintLintItem{
 				{Name: "Compliance", Description: "mountpoint /tmp required by policy is not present"},
 				{Name: "Compliance", Description: "mountpoint /var required by policy is not present"},
 				{Name: "Compliance", Description: "kernel command line parameter '-compliance' required by policy not set"},
 			},
+			lintWarnings: []v1.BlueprintLintItem{},
 		},
 		{
+			name: "filesystems and kernel params satisfied",
 			blueprint: v1.BlueprintBody{
 				Distribution: "rhel-8",
 				Customizations: v1.Customizations{
@@ -1570,57 +1583,130 @@ func TestLintBlueprint(t *testing.T) {
 					},
 				},
 			},
-			lintErrors: []v1.BlueprintLintItem{},
+			snapshot:     nil,
+			lintErrors:   []v1.BlueprintLintItem{},
+			lintWarnings: []v1.BlueprintLintItem{},
 		},
 		{
+			name: "unsupported minor version",
 			blueprint: v1.BlueprintBody{
 				Distribution: "rhel-89",
 				Customizations: v1.Customizations{
 					Openscap: &oscap2,
 				},
 			},
+			snapshot: nil,
 			lintErrors: []v1.BlueprintLintItem{
 				{Name: "Compliance", Description: "Compliance policy does not have a definition for the latest minor version"},
 			},
+			lintWarnings: []v1.BlueprintLintItem{},
 		},
 		{
+			name: "unsupported minor version duplicate",
 			blueprint: v1.BlueprintBody{
 				Distribution: "rhel-89",
 				Customizations: v1.Customizations{
 					Openscap: &oscap2,
 				},
 			},
+			snapshot: nil,
 			lintErrors: []v1.BlueprintLintItem{
 				// this error is unfixable for now
 				{Name: "Compliance", Description: "Compliance policy does not have a definition for the latest minor version"},
 			},
+			lintWarnings: []v1.BlueprintLintItem{},
 		},
 		{
+			name: "minimal policy missing package",
 			blueprint: v1.BlueprintBody{
 				Distribution: "rhel-8",
 				Customizations: v1.Customizations{
 					Openscap: &oscap3,
 				},
 			},
+			snapshot: nil,
 			lintErrors: []v1.BlueprintLintItem{
 				{Name: "Compliance", Description: "package required-by-compliance required by policy is not present"},
 			},
+			lintWarnings: []v1.BlueprintLintItem{},
+		},
+		{
+			name: "policy changes generate warnings",
+			blueprint: v1.BlueprintBody{
+				Distribution: "rhel-8",
+				Customizations: v1.Customizations{
+					Openscap: &oscap3, // Minimal policy - only requires "required-by-compliance" package
+					Packages: &[]string{
+						"required-by-compliance", // Still required by minimal policy
+					},
+				},
+			},
+			snapshot: func() *json.RawMessage {
+				// Create snapshot with previous policy's customizations (more than current policy requires)
+				snapshotData := map[string]interface{}{
+					"compliance": map[string]interface{}{
+						"policy_id": mocks.PolicyID, // Previous policy was the full one
+						"policy_customizations": map[string]interface{}{
+							"packages": []string{"required-by-compliance", "obsolete-package"},
+							"services": map[string]interface{}{
+								"enabled":  []string{"enabled-required-by-compliance", "obsolete-enabled-service"},
+								"masked":   []string{"masked-required-by-compliance", "obsolete-masked-service"},
+								"disabled": []string{"obsolete-disabled-service"},
+							},
+							"filesystem": []map[string]interface{}{
+								{"mountpoint": "/tmp", "min_size": 1000},
+								{"mountpoint": "/obsolete", "min_size": 500},
+							},
+							"kernel": map[string]interface{}{
+								"name":   "obsolete-kernel",
+								"append": "obsolete-param=1",
+							},
+							"fips": map[string]interface{}{
+								"enabled": true,
+							},
+						},
+					},
+				}
+				data, _ := json.Marshal(snapshotData)
+				rawMsg := json.RawMessage(data)
+				return &rawMsg
+			}(),
+			lintErrors: []v1.BlueprintLintItem{},
+			lintWarnings: []v1.BlueprintLintItem{
+				{Name: "Compliance", Description: "package obsolete-package is no longer required by policy"},
+				{Name: "Compliance", Description: "service enabled-required-by-compliance is no longer required as enabled by policy"},
+				{Name: "Compliance", Description: "service obsolete-enabled-service is no longer required as enabled by policy"},
+				{Name: "Compliance", Description: "service masked-required-by-compliance is no longer required as masked by policy"},
+				{Name: "Compliance", Description: "service obsolete-masked-service is no longer required as masked by policy"},
+				{Name: "Compliance", Description: "service obsolete-disabled-service is no longer required as disabled by policy"},
+				{Name: "Compliance", Description: "mountpoint /tmp is no longer required by policy"},
+				{Name: "Compliance", Description: "mountpoint /obsolete is no longer required by policy"},
+				{Name: "Compliance", Description: "kernel name obsolete-kernel is no longer required by policy"},
+				{Name: "Compliance", Description: "kernel command line parameter 'obsolete-param=1' is no longer required by policy"},
+				{Name: "Compliance", Description: "FIPS is no longer required by policy"},
+			},
 		},
 	}
 
 	for idx, c := range cases {
-		fmt.Printf("TestLintBlueprint case %d\n", idx)
+		fmt.Printf("TestLintBlueprint case %d: %s\n", idx, c.name)
 
 		bpID := uuid.New()
 		bpjson, err := json.Marshal(c.blueprint)
 		require.NoError(t, err)
-		require.NoError(t, srv.DB.InsertBlueprint(context.Background(), bpID, uuid.New(), "000000", "000000", "bp1", "", bpjson, nil, nil))
+
+		var snapshotBytes []byte
+		if c.snapshot != nil {
+			snapshotBytes = []byte(*c.snapshot)
+		}
+		require.NoError(t, srv.DB.InsertBlueprint(context.Background(), bpID, uuid.New(), "000000", "000000", "bp1", "", bpjson, nil, snapshotBytes))
 
 		var result v1.BlueprintResponse
 		respStatusCode, body := tutils.GetResponseBody(t, fmt.Sprintf("%s/api/image-builder/v1/blueprints/%s", srv.URL, bpID), &tutils.AuthString0)
 		require.Equal(t, http.StatusOK, respStatusCode)
 		require.NoError(t, json.Unmarshal([]byte(body), &result))
 		require.ElementsMatch(t, c.lintErrors, result.Lint.Errors)
+		require.ElementsMatch(t, c.lintWarnings, result.Lint.Warnings)
 
 		require.NoError(t, srv.DB.DeleteBlueprint(context.Background(), bpID, "000000"))
 	}

internal/v1/handler_oscap.go

@@ -241,18 +241,21 @@ func (h *Handlers) lintOpenscap(ctx echo.Context, bpBody *Customizations, fixup
 
 	policyBP, err := h.server.complianceClient.PolicyCustomizations(ctx.Request().Context(), major, minor, compl.PolicyId.String())
 	if err == compliance.ErrorTailoringNotFound {
+		if fixup {
+			return nil, nil, echo.NewHTTPError(http.StatusNotFound, err)
+		}
 		allErrors = append(allErrors, BlueprintLintItem{
 			Name:        "Compliance",
 			Description: "Compliance policy does not have a definition for the latest minor version",
 		})
 		policyBP = nil
 	} else if err != nil {
-		return nil, nil, err
+		return nil, nil, echo.NewHTTPError(http.StatusInternalServerError, err)
 	}
 
-	// If we couldn't find the policy and fixup was requested, return error early
+	// This should not happen if we handled errors correctly above
 	if policyBP == nil && fixup {
-		return nil, nil, echo.NewHTTPError(http.StatusNotFound, compliance.ErrorTailoringNotFound)
+		return nil, nil, echo.NewHTTPError(http.StatusInternalServerError, fmt.Errorf("unexpected nil policyBP during fixup"))
 	}
 
 	// Collect errors and warnings from all lint functions (they now return instead of mutating)

internal/v1/handler_oscap_test.go

@@ -540,4 +540,4 @@ func TestFIPS_DisabledInPolicy_SnapshotEnabled_Warns(t *testing.T) {
 	require.Len(t, warns, 1)
 	require.Equal(t, "Compliance", warns[0].Name)
 	require.Contains(t, warns[0].Description, "FIPS is no longer required by policy")
-}
+}