Skip to content

Commit 87cbe73

Browse files
exdxexdx
authored
Merge pull request #62 from exdx/feat/object-validation-checks
feat: bundle object validation
2 parents 00844b5 + 527bf81 commit 87cbe73

15 files changed

+412
-0
lines changed

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,5 @@
1616
*.DS_Store
1717
.AppleDouble
1818
.LSOverride
19+
20+
.idea/*

‎cmd/operator-verify/manifests/cmd.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ validation library.`,
2222
}
2323

2424
rootCmd.Flags().Bool("operatorhub_validate", false, "enable optional UI validation for operatorhub.io")
25+
rootCmd.Flags().Bool("object_validate", false, "enable optional bundle object validation")
2526

2627
return rootCmd
2728
}
@@ -40,10 +41,18 @@ func manifestsFunc(cmd *cobra.Command, args []string) {
4041
log.Fatalf("Unable to parse operatorhub_validate parameter")
4142
}
4243

44+
bundleObjectValidate, err := cmd.Flags().GetBool("object_validate")
45+
if err != nil {
46+
log.Fatalf("Unable to parse object_validate parameter: %w", err)
47+
}
48+
4349
validators := validation.DefaultBundleValidators
4450
if operatorHubValidate {
4551
validators = validators.WithValidators(validation.OperatorHubValidator)
4652
}
53+
if bundleObjectValidate {
54+
validators = validators.WithValidators(validation.ObjectValidator)
55+
}
4756

4857
results := validators.Validate(bundle.ObjectsToValidate()...)
4958
nonEmptyResults := []errors.ManifestResult{}

‎pkg/validation/errors/error.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ const (
100100
ErrorInvalidManifestStructure ErrorType = "ManifestStructureNotValid"
101101
ErrorInvalidBundle ErrorType = "BundleNotValid"
102102
ErrorInvalidPackageManifest ErrorType = "PackageManifestNotValid"
103+
ErrorObjectFailedValidation ErrorType = "ObjectFailedValidation"
103104
)
104105

105106
func NewError(t ErrorType, detail, field string, v interface{}) Error {
@@ -230,3 +231,15 @@ func WarnInvalidOperation(detail string, value interface{}) Error {
230231
func invalidOperation(lvl Level, detail string, value interface{}) Error {
231232
return Error{ErrorInvalidOperation, lvl, "", value, detail}
232233
}
234+
235+
func ErrInvalidObject(value interface{}, detail string) Error {
236+
return invalidObject(LevelError, detail, value)
237+
}
238+
239+
func invalidObject(lvl Level, detail string, value interface{}) Error {
240+
return Error{ErrorObjectFailedValidation, lvl, "", value, detail}
241+
}
242+
243+
func WarnInvalidObject(detail string, value interface{}) Error {
244+
return failedValidation(LevelWarn, detail, value)
245+
}

‎pkg/validation/internal/object.go‎

Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
package internal
2+
3+
import (
4+
"encoding/json"
5+
"github.com/operator-framework/api/pkg/validation/errors"
6+
interfaces "github.com/operator-framework/api/pkg/validation/interfaces"
7+
8+
policyv1beta1 "k8s.io/api/policy/v1beta1"
9+
rbacv1 "k8s.io/api/rbac/v1"
10+
schedulingv1 "k8s.io/api/scheduling/v1"
11+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
12+
)
13+
14+
var ObjectValidator interfaces.Validator = interfaces.ValidatorFunc(validateObjects)
15+
16+
const (
17+
PodDisruptionBudgetKind = "PodDisruptionBudget"
18+
PriorityClassKind = "PriorityClass"
19+
RoleKind = "Role"
20+
ClusterRoleKind = "ClusterRole"
21+
PodDisruptionBudgetAPIGroup = "policy"
22+
SCCAPIGroup = "security.openshift.io"
23+
)
24+
25+
// defaultSCCs is a map of the default Security Context Constraints present as of OpenShift 4.5.
26+
// See https://docs.openshift.com/container-platform/4.5/authentication/managing-security-context-constraints.html#security-context-constraints-about_configuring-internal-oauth
27+
var defaultSCCs = map[string]struct{}{
28+
"privileged": {},
29+
"restricted": {},
30+
"anyuid": {},
31+
"hostaccess": {},
32+
"hostmount-anyuid": {},
33+
"hostnetwork": {},
34+
"node-exporter": {},
35+
"nonroot": {},
36+
}
37+
38+
func validateObjects(objs ...interface{}) (results []errors.ManifestResult) {
39+
for _, obj := range objs {
40+
switch u := obj.(type) {
41+
case *unstructured.Unstructured:
42+
switch u.GroupVersionKind().Kind {
43+
case PodDisruptionBudgetKind:
44+
results = append(results, validatePDB(u))
45+
case PriorityClassKind:
46+
results = append(results, validatePriorityClass(u))
47+
case RoleKind:
48+
results = append(results, validateRBAC(u))
49+
case ClusterRoleKind:
50+
results = append(results, validateRBAC(u))
51+
}
52+
}
53+
}
54+
return results
55+
}
56+
57+
// validatePDB checks the PDB to ensure the minimum and maximum budgets are set to reasonable levels.
58+
// See https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/adding-pod-disruption-budgets.md#limitations-on-pod-disruption-budgets
59+
func validatePDB(u *unstructured.Unstructured) (result errors.ManifestResult) {
60+
pdb := policyv1beta1.PodDisruptionBudget{}
61+
62+
b, err := u.MarshalJSON()
63+
if err != nil {
64+
result.Add(errors.ErrInvalidParse("error converting unstructured", err))
65+
return
66+
}
67+
68+
err = json.Unmarshal(b, &pdb)
69+
if err != nil {
70+
result.Add(errors.ErrInvalidParse("error unmarshaling poddisruptionbudget", err))
71+
return
72+
}
73+
74+
/*
75+
maxUnavailable field cannot be set to 0 or 0%.
76+
minAvailable field cannot be set to 100%.
77+
*/
78+
79+
maxUnavailable := pdb.Spec.MaxUnavailable
80+
if maxUnavailable != nil && (maxUnavailable.IntVal == 0 || maxUnavailable.StrVal == "0%") {
81+
result.Add(errors.ErrInvalidObject(pdb, "maxUnavailable field cannot be set to 0 or 0%"))
82+
}
83+
84+
minAvailable := pdb.Spec.MinAvailable
85+
if minAvailable != nil && minAvailable.StrVal == "100%" {
86+
result.Add(errors.ErrInvalidObject(pdb, "minAvailable field cannot be set to 100%"))
87+
}
88+
89+
return
90+
}
91+
92+
// validatePriorityClass checks the PriorityClass object to ensure globalDefault is set to false.
93+
// See https://github.com/operator-framework/operator-lifecycle-manager/blob/master/doc/design/adding-priority-classes.md
94+
func validatePriorityClass(u *unstructured.Unstructured) (result errors.ManifestResult) {
95+
pc := schedulingv1.PriorityClass{}
96+
97+
b, err := u.MarshalJSON()
98+
if err != nil {
99+
result.Add(errors.ErrInvalidParse("error converting unstructured", err))
100+
return
101+
}
102+
103+
err = json.Unmarshal(b, &pc)
104+
if err != nil {
105+
result.Add(errors.ErrInvalidParse("error unmarshaling priorityclass", err))
106+
return
107+
}
108+
109+
if pc.GlobalDefault {
110+
result.Add(errors.ErrInvalidObject(pc, "globalDefault field cannot be set to true"))
111+
}
112+
113+
return
114+
}
115+
116+
func validateRBAC(u *unstructured.Unstructured) (result errors.ManifestResult) {
117+
var policyRules []rbacv1.PolicyRule
118+
119+
b, err := u.MarshalJSON()
120+
if err != nil {
121+
result.Add(errors.ErrInvalidParse("error converting unstructured", err))
122+
return
123+
}
124+
125+
switch u.GroupVersionKind().Kind {
126+
case RoleKind:
127+
role := rbacv1.Role{}
128+
err = json.Unmarshal(b, &role)
129+
if err != nil {
130+
result.Add(errors.ErrInvalidParse("error unmarshaling role", err))
131+
return
132+
}
133+
policyRules = role.Rules
134+
case ClusterRoleKind:
135+
clusterrole := rbacv1.ClusterRole{}
136+
err = json.Unmarshal(b, &clusterrole)
137+
if err != nil {
138+
result.Add(errors.ErrInvalidParse("error unmarshaling clusterrole", err))
139+
return
140+
}
141+
policyRules = clusterrole.Rules
142+
}
143+
144+
return audit(policyRules)
145+
}
146+
147+
// audit checks the provided rbac policies against prescribed limitations.
148+
// If permission is granted to create/modify a PDB, a warning is returned.
149+
// If permission is granted to modify default SCCs in OpenShift, an error is returned.
150+
func audit(policies []rbacv1.PolicyRule) (result errors.ManifestResult) {
151+
// check for permission to modify/create PDBs
152+
for _, rule := range policies {
153+
if contains(rule.APIGroups, PodDisruptionBudgetAPIGroup) &&
154+
contains(rule.Resources, "poddisruptionbudgets") &&
155+
contains(rule.Verbs, rbacv1.VerbAll, "create", "update", "patch") {
156+
result.Add(errors.WarnInvalidObject("RBAC includes permission to create/update poddisruptionbudgets, which could impact cluster stability", rule))
157+
}
158+
}
159+
160+
// check sccs for modifying default known SCCs
161+
for _, rule := range policies {
162+
if contains(rule.APIGroups, SCCAPIGroup) &&
163+
contains(rule.Resources, "securitycontextconstraints") &&
164+
contains(rule.Verbs, rbacv1.VerbAll, "delete", "update", "patch") &&
165+
containsDefaults(rule.ResourceNames, defaultSCCs) {
166+
result.Add(errors.ErrInvalidObject(rule, "RBAC includes permission to modify default securitycontextconstraints, which could impact cluster stability"))
167+
}
168+
}
169+
170+
return
171+
}
172+
173+
// contains returns true if at least one item is present in the array
174+
func contains(slice []string, items ...string) bool {
175+
set := make(map[string]struct{}, len(slice))
176+
for _, s := range slice {
177+
set[s] = struct{}{}
178+
}
179+
180+
for _, item := range items {
181+
if _, ok := set[item]; ok {
182+
return true
183+
}
184+
}
185+
186+
return false
187+
}
188+
189+
// containsDefaults returns true if at least one item is present as a key in the map
190+
func containsDefaults(slice []string, defaults map[string]struct{}) bool {
191+
for _, s := range slice {
192+
if _, ok := defaults[s]; ok {
193+
return true
194+
}
195+
}
196+
return false
197+
}

‎pkg/validation/internal/object_test.go‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package internal
2+
3+
import (
4+
"github.com/ghodss/yaml"
5+
"io/ioutil"
6+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
7+
"testing"
8+
)
9+
10+
func TestValidateObject(t *testing.T) {
11+
var table = []struct {
12+
description string
13+
path string
14+
error bool
15+
warning bool
16+
detail string
17+
}{
18+
{
19+
description: "valid PDB",
20+
path: "./testdata/objects/valid_pdb.yaml",
21+
},
22+
{
23+
description: "invalid PDB - minAvailable set to 100%",
24+
path: "./testdata/objects/invalid_pdb_minAvailable.yaml",
25+
error: true,
26+
detail: "minAvailable field cannot be set to 100%",
27+
},
28+
{
29+
description: "invalid PDB - maxUnavailable set to 0",
30+
path: "./testdata/objects/invalid_pdb_maxUnavailable.yaml",
31+
error: true,
32+
detail: "maxUnavailable field cannot be set to 0 or 0%",
33+
},
34+
{
35+
description: "valid priorityclass",
36+
path: "./testdata/objects/valid_priorityclass.yaml",
37+
},
38+
{
39+
description: "invalid priorityclass - global default set to true",
40+
path: "./testdata/objects/invalid_priorityclass.yaml",
41+
error: true,
42+
detail: "globalDefault field cannot be set to true",
43+
},
44+
{
45+
description: "valid pdb role",
46+
path: "./testdata/objects/valid_role_get_pdb.yaml",
47+
},
48+
{
49+
description: "invalid role - modify pdb",
50+
path: "./testdata/objects/invalid_role_create_pdb.yaml",
51+
warning: true,
52+
detail: "RBAC includes permission to create/update poddisruptionbudgets, which could impact cluster stability",
53+
},
54+
{
55+
description: "valid scc role",
56+
path: "./testdata/objects/valid_role_get_scc.yaml",
57+
},
58+
{
59+
description: "invalid scc role - modify default scc",
60+
path: "./testdata/objects/invalid_role_modify_scc.yaml",
61+
error: true,
62+
detail: "RBAC includes permission to modify default securitycontextconstraints, which could impact cluster stability",
63+
},
64+
}
65+
66+
for _, tt := range table {
67+
t.Log(tt.description)
68+
69+
u := unstructured.Unstructured{}
70+
o, err := ioutil.ReadFile(tt.path)
71+
if err != nil {
72+
t.Fatalf("reading yaml object file: %s", err)
73+
}
74+
if err := yaml.Unmarshal(o, &u); err != nil {
75+
t.Fatalf("unmarshalling object at path %s: %v", tt.path, err)
76+
}
77+
78+
results := ObjectValidator.Validate(&u)
79+
80+
// check errors
81+
if len(results[0].Errors) > 0 && tt.error == false {
82+
t.Fatalf("received errors %#v when no validation error expected for %s", results, tt.path)
83+
}
84+
if len(results[0].Errors) == 0 && tt.error == true {
85+
t.Fatalf("received no errors when validation error expected for %s", tt.path)
86+
}
87+
if len(results[0].Errors) > 0 {
88+
if results[0].Errors[0].Detail != tt.detail {
89+
t.Fatalf("expected validation error detail %s, got %s", tt.detail, results[0].Errors[0].Detail)
90+
}
91+
}
92+
93+
// check warnings
94+
if len(results[0].Warnings) > 0 && tt.warning == false {
95+
t.Fatalf("received errors %#v when no validation warning expected for %s", results, tt.path)
96+
}
97+
if len(results[0].Warnings) == 0 && tt.warning == true {
98+
t.Fatalf("received no errors when validation warning expected for %s", tt.path)
99+
}
100+
if len(results[0].Warnings) > 0 {
101+
if results[0].Warnings[0].Detail != tt.detail {
102+
t.Fatalf("expected validation warning detail %s, got %s", tt.detail, results[0].Warnings[0].Detail)
103+
}
104+
}
105+
}
106+
107+
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
apiVersion: policy/v1beta1
2+
kind: PodDisruptionBudget
3+
metadata:
4+
name: busybox-pdb
5+
spec:
6+
maxUnavailable: 0

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
apiVersion: policy/v1beta1
2+
kind: PodDisruptionBudget
3+
metadata:
4+
name: busybox-pdb
5+
spec:
6+
minAvailable: 100%

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
apiVersion: scheduling.k8s.io/v1
2+
kind: PriorityClass
3+
metadata:
4+
name: super-priority
5+
value: 1000
6+
globalDefault: true

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
apiVersion: rbac.authorization.k8s.io/v1
2+
kind: Role
3+
metadata:
4+
namespace: default
5+
name: pdb-modifier
6+
rules:
7+
- apiGroups: ["policy"]
8+
resources: ["poddisruptionbudgets"]
9+
verbs: ["create", "list"]

0 commit comments

Comments
 (0)