feat: bundle object validation

.gitignore

@@ -16,3 +16,5 @@
 *.DS_Store
 .AppleDouble
 .LSOverride
+
+.idea/*

cmd/operator-verify/manifests/cmd.go

@@ -22,6 +22,7 @@ validation library.`,
 	}
 
 	rootCmd.Flags().Bool("operatorhub_validate", false, "enable optional UI validation for operatorhub.io")
+	rootCmd.Flags().Bool("object_validate", false, "enable optional bundle object validation")
 
 	return rootCmd
 }
@@ -40,10 +41,18 @@ func manifestsFunc(cmd *cobra.Command, args []string) {
 		log.Fatalf("Unable to parse operatorhub_validate parameter")
 	}
 
+	bundleObjectValidate, err := cmd.Flags().GetBool("object_validate")
+	if err != nil {
+		log.Fatalf("Unable to parse object_validate parameter: %w", err)
+	}
+
 	validators := validation.DefaultBundleValidators
 	if operatorHubValidate {
 		validators = validators.WithValidators(validation.OperatorHubValidator)
 	}
+	if bundleObjectValidate {
+		validators = validators.WithValidators(validation.ObjectValidator)
+	}
 
 	results := validators.Validate(bundle.ObjectsToValidate()...)
 	nonEmptyResults := []errors.ManifestResult{}

pkg/validation/errors/error.go

@@ -100,6 +100,7 @@ const (
 	ErrorInvalidManifestStructure ErrorType = "ManifestStructureNotValid"
 	ErrorInvalidBundle            ErrorType = "BundleNotValid"
 	ErrorInvalidPackageManifest   ErrorType = "PackageManifestNotValid"
+	ErrorObjectFailedValidation   ErrorType = "ObjectFailedValidation"
 )
 
 func NewError(t ErrorType, detail, field string, v interface{}) Error {
@@ -230,3 +231,15 @@ func WarnInvalidOperation(detail string, value interface{}) Error {
 func invalidOperation(lvl Level, detail string, value interface{}) Error {
 	return Error{ErrorInvalidOperation, lvl, "", value, detail}
 }
+
+func ErrInvalidObject(value interface{}, detail string) Error {
+	return invalidObject(LevelError, detail, value)
+}
+
+func invalidObject(lvl Level, detail string, value interface{}) Error {
+	return Error{ErrorObjectFailedValidation, lvl, "", value, detail}
+}
+
+func WarnInvalidObject(detail string, value interface{}) Error {
+	return failedValidation(LevelWarn, detail, value)
+}

pkg/validation/internal/object.go

@@ -0,0 +1,197 @@
+package internal
+
+import (
+	"encoding/json"
+	"github.com/operator-framework/api/pkg/validation/errors"
+	interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
+
+	policyv1beta1 "k8s.io/api/policy/v1beta1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	schedulingv1 "k8s.io/api/scheduling/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+)
+
+var ObjectValidator interfaces.Validator = interfaces.ValidatorFunc(validateObjects)
+
+const (
+	PodDisruptionBudgetKind     = "PodDisruptionBudget"
+	PriorityClassKind           = "PriorityClass"
+	RoleKind                    = "Role"
+	ClusterRoleKind             = "ClusterRole"
+	PodDisruptionBudgetAPIGroup = "policy"
+	SCCAPIGroup                 = "security.openshift.io"
+)
+
+// defaultSCCs is a map of the default Security Context Constraints present as of OpenShift 4.5.
+// See https://docs.openshift.com/container-platform/4.5/authentication/managing-security-context-constraints.html#security-context-constraints-about_configuring-internal-oauth
+var defaultSCCs = map[string]struct{}{
+	"privileged":       {},
+	"restricted":       {},
+	"anyuid":           {},
+	"hostaccess":       {},
+	"hostmount-anyuid": {},
+	"hostnetwork":      {},
+	"node-exporter":    {},
+	"nonroot":          {},
+}
+
+func validateObjects(objs ...interface{}) (results []errors.ManifestResult) {
+	for _, obj := range objs {
+		switch u := obj.(type) {
+		case *unstructured.Unstructured:
+			switch u.GroupVersionKind().Kind {
+			case PodDisruptionBudgetKind:
+				results = append(results, validatePDB(u))
+			case PriorityClassKind:
+				results = append(results, validatePriorityClass(u))
+			case RoleKind:
+				results = append(results, validateRBAC(u))
+			case ClusterRoleKind:
+				results = append(results, validateRBAC(u))
+			}
+		}
+	}
+	return results
+}
+
+// validatePDB checks the PDB to ensure the minimum and maximum budgets are set to reasonable levels.
+// See https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/adding-pod-disruption-budgets.md#limitations-on-pod-disruption-budgets
+func validatePDB(u *unstructured.Unstructured) (result errors.ManifestResult) {
+	pdb := policyv1beta1.PodDisruptionBudget{}
+
+	b, err := u.MarshalJSON()
+	if err != nil {
+		result.Add(errors.ErrInvalidParse("error converting unstructured", err))
+		return
+	}
+
+	err = json.Unmarshal(b, &pdb)
+	if err != nil {
+		result.Add(errors.ErrInvalidParse("error unmarshaling poddisruptionbudget", err))
+		return
+	}
+
+	/*
+	   maxUnavailable field cannot be set to 0 or 0%.
+	   minAvailable field cannot be set to 100%.
+	*/
+
+	maxUnavailable := pdb.Spec.MaxUnavailable
+	if maxUnavailable != nil && (maxUnavailable.IntVal == 0 || maxUnavailable.StrVal == "0%") {
+		result.Add(errors.ErrInvalidObject(pdb, "maxUnavailable field cannot be set to 0 or 0%"))
+	}
+
+	minAvailable := pdb.Spec.MinAvailable
+	if minAvailable != nil && minAvailable.StrVal == "100%" {
+		result.Add(errors.ErrInvalidObject(pdb, "minAvailable field cannot be set to 100%"))
+	}
+
+	return
+}
+
+// validatePriorityClass checks the PriorityClass object to ensure globalDefault is set to false.
+// See https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/adding-priority-classes.md
+func validatePriorityClass(u *unstructured.Unstructured) (result errors.ManifestResult) {
+	pc := schedulingv1.PriorityClass{}
+
+	b, err := u.MarshalJSON()
+	if err != nil {
+		result.Add(errors.ErrInvalidParse("error converting unstructured", err))
+		return
+	}
+
+	err = json.Unmarshal(b, &pc)
+	if err != nil {
+		result.Add(errors.ErrInvalidParse("error unmarshaling priorityclass", err))
+		return
+	}
+
+	if pc.GlobalDefault {
+		result.Add(errors.ErrInvalidObject(pc, "globalDefault field cannot be set to true"))
+	}
+
+	return
+}
+
+func validateRBAC(u *unstructured.Unstructured) (result errors.ManifestResult) {
+	var policyRules []rbacv1.PolicyRule
+
+	b, err := u.MarshalJSON()
+	if err != nil {
+		result.Add(errors.ErrInvalidParse("error converting unstructured", err))
+		return
+	}
+
+	switch u.GroupVersionKind().Kind {
+	case RoleKind:
+		role := rbacv1.Role{}
+		err = json.Unmarshal(b, &role)
+		if err != nil {
+			result.Add(errors.ErrInvalidParse("error unmarshaling role", err))
+			return
+		}
+		policyRules = role.Rules
+	case ClusterRoleKind:
+		clusterrole := rbacv1.ClusterRole{}
+		err = json.Unmarshal(b, &clusterrole)
+		if err != nil {
+			result.Add(errors.ErrInvalidParse("error unmarshaling clusterrole", err))
+			return
+		}
+		policyRules = clusterrole.Rules
+	}
+
+	return audit(policyRules)
+}
+
+// audit checks the provided rbac policies against prescribed limitations.
+// If permission is granted to create/modify a PDB, a warning is returned.
+// If permission is granted to modify default SCCs in OpenShift, an error is returned.
+func audit(policies []rbacv1.PolicyRule) (result errors.ManifestResult) {
+	// check for permission to modify/create PDBs
+	for _, rule := range policies {
+		if contains(rule.APIGroups, PodDisruptionBudgetAPIGroup) &&
+			contains(rule.Resources, "poddisruptionbudgets") &&
+			contains(rule.Verbs, rbacv1.VerbAll, "create", "update", "patch") {
+			result.Add(errors.WarnInvalidObject("RBAC includes permission to create/update poddisruptionbudgets, which could impact cluster stability", rule))
+		}
+	}
+
+	// check sccs for modifying default known SCCs
+	for _, rule := range policies {
+		if contains(rule.APIGroups, SCCAPIGroup) &&
+			contains(rule.Resources, "securitycontextconstraints") &&
+			contains(rule.Verbs, rbacv1.VerbAll, "delete", "update", "patch") &&
+			containsDefaults(rule.ResourceNames, defaultSCCs) {
+			result.Add(errors.ErrInvalidObject(rule, "RBAC includes permission to modify default securitycontextconstraints, which could impact cluster stability"))
+		}
+	}
+
+	return
+}
+
+// contains returns true if at least one item is present in the array
+func contains(slice []string, items ...string) bool {
+	set := make(map[string]struct{}, len(slice))
+	for _, s := range slice {
+		set[s] = struct{}{}
+	}
+
+	for _, item := range items {
+		if _, ok := set[item]; ok {
+			return true
+		}
+	}
+
+	return false
+}
+
+// containsDefaults returns true if at least one item is present as a key in the map
+func containsDefaults(slice []string, defaults map[string]struct{}) bool {
+	for _, s := range slice {
+		if _, ok := defaults[s]; ok {
+			return true
+		}
+	}
+	return false
+}

pkg/validation/internal/object_test.go

@@ -0,0 +1,107 @@
+package internal
+
+import (
+	"github.com/ghodss/yaml"
+	"io/ioutil"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"testing"
+)
+
+func TestValidateObject(t *testing.T) {
+	var table = []struct {
+		description string
+		path        string
+		error       bool
+		warning     bool
+		detail      string
+	}{
+		{
+			description: "valid PDB",
+			path:        "./testdata/objects/valid_pdb.yaml",
+		},
+		{
+			description: "invalid PDB - minAvailable set to 100%",
+			path:        "./testdata/objects/invalid_pdb_minAvailable.yaml",
+			error:       true,
+			detail:      "minAvailable field cannot be set to 100%",
+		},
+		{
+			description: "invalid PDB - maxUnavailable set to 0",
+			path:        "./testdata/objects/invalid_pdb_maxUnavailable.yaml",
+			error:       true,
+			detail:      "maxUnavailable field cannot be set to 0 or 0%",
+		},
+		{
+			description: "valid priorityclass",
+			path:        "./testdata/objects/valid_priorityclass.yaml",
+		},
+		{
+			description: "invalid priorityclass - global default set to true",
+			path:        "./testdata/objects/invalid_priorityclass.yaml",
+			error:       true,
+			detail:      "globalDefault field cannot be set to true",
+		},
+		{
+			description: "valid pdb role",
+			path:        "./testdata/objects/valid_role_get_pdb.yaml",
+		},
+		{
+			description: "invalid role - modify pdb",
+			path:        "./testdata/objects/invalid_role_create_pdb.yaml",
+			warning:     true,
+			detail:      "RBAC includes permission to create/update poddisruptionbudgets, which could impact cluster stability",
+		},
+		{
+			description: "valid scc role",
+			path:        "./testdata/objects/valid_role_get_scc.yaml",
+		},
+		{
+			description: "invalid scc role - modify default scc",
+			path:        "./testdata/objects/invalid_role_modify_scc.yaml",
+			error:       true,
+			detail:      "RBAC includes permission to modify default securitycontextconstraints, which could impact cluster stability",
+		},
+	}
+
+	for _, tt := range table {
+		t.Log(tt.description)
+
+		u := unstructured.Unstructured{}
+		o, err := ioutil.ReadFile(tt.path)
+		if err != nil {
+			t.Fatalf("reading yaml object file: %s", err)
+		}
+		if err := yaml.Unmarshal(o, &u); err != nil {
+			t.Fatalf("unmarshalling object at path %s: %v", tt.path, err)
+		}
+
+		results := ObjectValidator.Validate(&u)
+
+		// check errors
+		if len(results[0].Errors) > 0 && tt.error == false {
+			t.Fatalf("received errors %#v when no validation error expected for %s", results, tt.path)
+		}
+		if len(results[0].Errors) == 0 && tt.error == true {
+			t.Fatalf("received no errors when validation error expected for %s", tt.path)
+		}
+		if len(results[0].Errors) > 0 {
+			if results[0].Errors[0].Detail != tt.detail {
+				t.Fatalf("expected validation error detail %s, got %s", tt.detail, results[0].Errors[0].Detail)
+			}
+		}
+
+		// check warnings
+		if len(results[0].Warnings) > 0 && tt.warning == false {
+			t.Fatalf("received errors %#v when no validation warning expected for %s", results, tt.path)
+		}
+		if len(results[0].Warnings) == 0 && tt.warning == true {
+			t.Fatalf("received no errors when validation warning expected for %s", tt.path)
+		}
+		if len(results[0].Warnings) > 0 {
+			if results[0].Warnings[0].Detail != tt.detail {
+				t.Fatalf("expected validation warning detail %s, got %s", tt.detail, results[0].Warnings[0].Detail)
+			}
+		}
+	}
+
+}

pkg/validation/internal/testdata/objects/invalid_pdb_maxUnavailable.yaml

@@ -0,0 +1,6 @@
+apiVersion: policy/v1beta1
+kind: PodDisruptionBudget
+metadata:
+  name: busybox-pdb
+spec:
+  maxUnavailable: 0

pkg/validation/internal/testdata/objects/invalid_pdb_minAvailable.yaml

@@ -0,0 +1,6 @@
+apiVersion: policy/v1beta1
+kind: PodDisruptionBudget
+metadata:
+  name: busybox-pdb
+spec:
+  minAvailable: 100%

pkg/validation/internal/testdata/objects/invalid_priorityclass.yaml

@@ -0,0 +1,6 @@
+apiVersion: scheduling.k8s.io/v1
+kind: PriorityClass
+metadata:
+  name: super-priority
+value: 1000
+globalDefault: true

pkg/validation/internal/testdata/objects/invalid_role_create_pdb.yaml

@@ -0,0 +1,9 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  namespace: default
+  name: pdb-modifier
+rules:
+  - apiGroups: ["policy"]
+    resources: ["poddisruptionbudgets"]
+    verbs: ["create", "list"]