From f2daa6131f3f0c96b18fba33172c5e587fc8ca61 Mon Sep 17 00:00:00 2001 From: Dustin Scott Date: Mon, 7 Mar 2022 15:01:16 -0600 Subject: [PATCH] feat: add rbac rules for non-resource urls (#279) * refactor: move rbac into its own package Signed-off-by: Dustin Scott * fix: Fixes #274, added capability to generate rules for nonresource urls Signed-off-by: Dustin Scott * refactor: move utils file into proper utils package Signed-off-by: Dustin Scott * test: added test cases for rbac after refactor Signed-off-by: Dustin Scott * chore: fix linting Signed-off-by: Dustin Scott * test: fix test case for roles with non-resource urls Signed-off-by: Dustin Scott --- internal/utils/conversion.go | 78 +++ .../{workload/v1/utils.go => utils/files.go} | 63 +- internal/workload/v1/collection.go | 7 +- internal/workload/v1/component.go | 7 +- internal/workload/v1/config.go | 11 +- internal/workload/v1/init_config.go | 10 +- internal/workload/v1/interfaces.go | 8 +- internal/workload/v1/rbac.go | 364 ----------- internal/workload/v1/rbac/rbac.go | 149 +++++ .../workload/v1/rbac/rbac_internal_test.go | 262 ++++++++ internal/workload/v1/rbac/role_rule.go | 125 ++++ .../v1/rbac/role_rule_internal_test.go | 359 +++++++++++ internal/workload/v1/rbac/rule.go | 135 ++++ .../workload/v1/rbac/rule_internal_test.go | 577 ++++++++++++++++++ internal/workload/v1/rbac/rules.go | 106 ++++ .../workload/v1/rbac/rules_internal_test.go | 293 +++++++++ internal/workload/v1/rbac_internal_test.go | 51 -- internal/workload/v1/resource.go | 2 +- internal/workload/v1/standalone.go | 7 +- internal/workload/v1/workload.go | 54 +- .../ingress/contour-component.yaml | 1 + .../.workloadConfig/ingress/rbac.yaml | 21 + 22 files changed, 2161 insertions(+), 529 deletions(-) create mode 100644 internal/utils/conversion.go rename internal/{workload/v1/utils.go => utils/files.go} (62%) delete mode 100644 internal/workload/v1/rbac.go create mode 100644 internal/workload/v1/rbac/rbac.go create mode 100644 internal/workload/v1/rbac/rbac_internal_test.go create mode 100644 internal/workload/v1/rbac/role_rule.go create mode 100644 internal/workload/v1/rbac/role_rule_internal_test.go create mode 100644 internal/workload/v1/rbac/rule.go create mode 100644 internal/workload/v1/rbac/rule_internal_test.go create mode 100644 internal/workload/v1/rbac/rules.go create mode 100644 internal/workload/v1/rbac/rules_internal_test.go delete mode 100644 internal/workload/v1/rbac_internal_test.go create mode 100644 test/cases/edge-collection/.workloadConfig/ingress/rbac.yaml diff --git a/internal/utils/conversion.go b/internal/utils/conversion.go new file mode 100644 index 0000000..8c99997 --- /dev/null +++ b/internal/utils/conversion.go @@ -0,0 +1,78 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: MIT + +package utils + +import ( + "errors" + "fmt" +) + +var ( + ErrConvertArrayInterface = errors.New("unable to convert to []interface{}") + ErrConvertArrayString = errors.New("unable to convert to []string") + ErrConvertMapStringInterface = errors.New("unable to convert to map[string]interface{}") + ErrConvertString = errors.New("unable to convert to string") +) + +// ToArrayInterface attempts a conversion from an interface to an underlying array of +// interface type. Returns an error if the conversion is not possible. +func ToArrayInterface(in interface{}) ([]interface{}, error) { + out, ok := in.([]interface{}) + if !ok { + return nil, ErrConvertArrayInterface + } + + return out, nil +} + +// ToArrayString attempts a conversion from an interface to an underlying array of +// string type. Returns an error if the conversion is not possible. +func ToArrayString(in interface{}) ([]string, error) { + // attempt direct conversion + out, ok := in.([]string) + if !ok { + // attempt conversion for each item + outInterfaces, err := ToArrayInterface(in) + if err != nil { + return nil, fmt.Errorf("%w; %s", err, ErrConvertArrayString) + } + + outStrings := make([]string, len(outInterfaces)) + + for i := range outInterfaces { + outString, err := ToString(outInterfaces[i]) + if err != nil { + return nil, fmt.Errorf("%w; %s", err, ErrConvertArrayString) + } + + outStrings[i] = outString + } + + return outStrings, nil + } + + return out, nil +} + +// ToMapStringInterface attempts a conversion from an interface to an underlying map +// string interface type. Returns an error if the conversion is not possible. +func ToMapStringInterface(in interface{}) (map[string]interface{}, error) { + out, ok := in.(map[string]interface{}) + if !ok { + return nil, ErrConvertMapStringInterface + } + + return out, nil +} + +// ToArrayInterface attempts a conversion from an interface to an underlying +// string type. Returns an error if the conversion is not possible. +func ToString(in interface{}) (string, error) { + out, ok := in.(string) + if !ok { + return "", ErrConvertString + } + + return out, nil +} diff --git a/internal/workload/v1/utils.go b/internal/utils/files.go similarity index 62% rename from internal/workload/v1/utils.go rename to internal/utils/files.go index 534e964..18adb9a 100644 --- a/internal/workload/v1/utils.go +++ b/internal/utils/files.go @@ -1,7 +1,7 @@ // Copyright 2021 VMware, Inc. // SPDX-License-Identifier: MIT -package v1 +package utils import ( "errors" @@ -13,13 +13,6 @@ import ( "strings" ) -var ( - ErrConvertArrayInterface = errors.New("unable to convert to []interface{}") - ErrConvertArrayString = errors.New("unable to convert to []string") - ErrConvertMapStringInterface = errors.New("unable to convert to map[string]interface{}") - ErrConvertString = errors.New("unable to convert to string") -) - func ReadStream(fileName string) (io.ReadCloser, error) { file, err := os.Open(fileName) if err != nil { @@ -109,57 +102,3 @@ func expand(g []string) ([]string, error) { return matches, nil } - -func toArrayInterface(in interface{}) ([]interface{}, error) { - out, ok := in.([]interface{}) - if !ok { - return nil, ErrConvertArrayInterface - } - - return out, nil -} - -func toArrayString(in interface{}) ([]string, error) { - // attempt direct conversion - out, ok := in.([]string) - if !ok { - // attempt conversion for each item - outInterfaces, err := toArrayInterface(in) - if err != nil { - return nil, fmt.Errorf("%w; %s", err, ErrConvertArrayString) - } - - outStrings := make([]string, len(outInterfaces)) - - for i := range outInterfaces { - outString, err := toString(outInterfaces[i]) - if err != nil { - return nil, fmt.Errorf("%w; %s", err, ErrConvertArrayString) - } - - outStrings[i] = outString - } - - return outStrings, nil - } - - return out, nil -} - -func toMapStringInterface(in interface{}) (map[string]interface{}, error) { - out, ok := in.(map[string]interface{}) - if !ok { - return nil, ErrConvertMapStringInterface - } - - return out, nil -} - -func toString(in interface{}) (string, error) { - out, ok := in.(string) - if !ok { - return "", ErrConvertString - } - - return out, nil -} diff --git a/internal/workload/v1/collection.go b/internal/workload/v1/collection.go index c129225..3eb2aa9 100644 --- a/internal/workload/v1/collection.go +++ b/internal/workload/v1/collection.go @@ -11,6 +11,7 @@ import ( "github.com/vmware-tanzu-labs/operator-builder/internal/utils" "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/markers" + "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/rbac" ) const ( @@ -152,7 +153,7 @@ func (c *WorkloadCollection) IsCollection() bool { } func (c *WorkloadCollection) SetRBAC() { - c.Spec.RBACRules.addRulesForWorkload(c) + c.Spec.RBACRules.Add(rbac.ForWorkloads(c)) } func (c *WorkloadCollection) SetResources(workloadPath string) error { @@ -208,8 +209,8 @@ func (c *WorkloadCollection) GetAPISpecFields() *APIFields { return c.Spec.APISpecFields } -func (c *WorkloadCollection) GetRBACRules() *[]RBACRule { - var rules []RBACRule = *c.Spec.RBACRules +func (c *WorkloadCollection) GetRBACRules() *[]rbac.Rule { + var rules []rbac.Rule = *c.Spec.RBACRules return &rules } diff --git a/internal/workload/v1/component.go b/internal/workload/v1/component.go index 90380d5..cc92048 100644 --- a/internal/workload/v1/component.go +++ b/internal/workload/v1/component.go @@ -11,6 +11,7 @@ import ( "github.com/vmware-tanzu-labs/operator-builder/internal/utils" "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/markers" + "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/rbac" ) var ErrNoComponentsOnComponent = errors.New("cannot set component workloads on a component workload - only on collections") @@ -134,7 +135,7 @@ func (*ComponentWorkload) IsCollection() bool { } func (c *ComponentWorkload) SetRBAC() { - c.Spec.RBACRules.addRulesForWorkload(c) + c.Spec.RBACRules.Add(rbac.ForWorkloads(c, c.Spec.Collection)) } func (c *ComponentWorkload) SetResources(workloadPath string) error { @@ -178,8 +179,8 @@ func (c *ComponentWorkload) GetAPISpecFields() *APIFields { return c.Spec.APISpecFields } -func (c *ComponentWorkload) GetRBACRules() *[]RBACRule { - var rules []RBACRule = *c.Spec.RBACRules +func (c *ComponentWorkload) GetRBACRules() *[]rbac.Rule { + var rules []rbac.Rule = *c.Spec.RBACRules return &rules } diff --git a/internal/workload/v1/config.go b/internal/workload/v1/config.go index f1b0947..c704cc9 100644 --- a/internal/workload/v1/config.go +++ b/internal/workload/v1/config.go @@ -13,6 +13,7 @@ import ( "github.com/go-playground/validator" "gopkg.in/yaml.v3" + "github.com/vmware-tanzu-labs/operator-builder/internal/utils" "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/markers" ) @@ -182,12 +183,12 @@ func parseConfig(workloadConfig string) (map[WorkloadKind][]WorkloadIdentifier, return nil, ErrConfigMustExist } - file, err := ReadStream(workloadConfig) + file, err := utils.ReadStream(workloadConfig) if err != nil { - return nil, err + return nil, fmt.Errorf("%w; error reading file %s", err, workloadConfig) } - defer CloseFile(file) + defer utils.CloseFile(file) var kindReader bytes.Buffer reader := io.TeeReader(file, &kindReader) @@ -252,9 +253,9 @@ func parseCollectionComponents(workload *WorkloadCollection, workloadConfig stri var workloads []WorkloadIdentifier for _, componentFile := range workload.Spec.ComponentFiles { - componentPaths, err := Glob(filepath.Join(filepath.Dir(workloadConfig), componentFile)) + componentPaths, err := utils.Glob(filepath.Join(filepath.Dir(workloadConfig), componentFile)) if err != nil { - return nil, err + return nil, fmt.Errorf("%w; error globbing workload config at path %s", err, componentFile) } for _, componentPath := range componentPaths { diff --git a/internal/workload/v1/init_config.go b/internal/workload/v1/init_config.go index c4ca544..4a8263a 100644 --- a/internal/workload/v1/init_config.go +++ b/internal/workload/v1/init_config.go @@ -10,6 +10,8 @@ import ( "os" "gopkg.in/yaml.v3" + + "github.com/vmware-tanzu-labs/operator-builder/internal/utils" ) var ( @@ -117,12 +119,12 @@ func mutateConfig(workloadConfig WorkloadInitializer) ([]byte, error) { } func mutateResources(yamlData map[string]interface{}) error { - specField, err := toMapStringInterface(yamlData["spec"]) + specField, err := utils.ToMapStringInterface(yamlData["spec"]) if err != nil { return fmt.Errorf("%w; error converting workload config spec %v", err, yamlData["spec"]) } - resourceObjs, err := toArrayInterface(specField["resources"]) + resourceObjs, err := utils.ToArrayInterface(specField["resources"]) if err != nil { return fmt.Errorf("%w; error converting spec.resources %v", err, specField["resources"]) } @@ -130,12 +132,12 @@ func mutateResources(yamlData map[string]interface{}) error { resources := make([]string, len(resourceObjs)) for i, resource := range resourceObjs { - resourceMap, err := toMapStringInterface(resource) + resourceMap, err := utils.ToMapStringInterface(resource) if err != nil { return fmt.Errorf("%w; error converting spec.resources item %v", err, resource) } - filename, err := toString(resourceMap["filename"]) + filename, err := utils.ToString(resourceMap["filename"]) if err != nil { return fmt.Errorf("%w; error converting spec.resources.filename %v", err, resource) } diff --git a/internal/workload/v1/interfaces.go b/internal/workload/v1/interfaces.go index 7a32c7b..bb3015e 100644 --- a/internal/workload/v1/interfaces.go +++ b/internal/workload/v1/interfaces.go @@ -3,7 +3,11 @@ package v1 -import "sigs.k8s.io/kubebuilder/v3/pkg/model/resource" +import ( + "sigs.k8s.io/kubebuilder/v3/pkg/model/resource" + + "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/rbac" +) // WorkloadIdentifier defines an interface for identifying any workload. type WorkloadIdentifier interface { @@ -51,7 +55,7 @@ type WorkloadAPIBuilder interface { GetComponents() []*ComponentWorkload GetSourceFiles() *[]SourceFile GetAPISpecFields() *APIFields - GetRBACRules() *[]RBACRule + GetRBACRules() *[]rbac.Rule GetComponentResource(domain, repo string, clusterScoped bool) *resource.Resource GetFuncNames() (createFuncNames, initFuncNames []string) GetRootCommand() *CliCommand diff --git a/internal/workload/v1/rbac.go b/internal/workload/v1/rbac.go deleted file mode 100644 index 8036751..0000000 --- a/internal/workload/v1/rbac.go +++ /dev/null @@ -1,364 +0,0 @@ -// Copyright 2021 VMware, Inc. -// SPDX-License-Identifier: MIT - -package v1 - -import ( - "fmt" - "strings" - - "sigs.k8s.io/kubebuilder/v3/pkg/model/resource" -) - -const ( - coreRBACGroup = "core" -) - -// RBACRule contains the info needed to create the kubebuilder:rbac markers in -// the controller. -type RBACRule struct { - Group string - Resource string - Verbs []string - URLs []string -} - -// RBACRoleRule contains the info needed to create the kubebuilder:rbac markers -// in the controller when a resource that is of a role or clusterrole type is -// found. This is because the underlying controller needs the same permissions -// for the role or clusterrole that it is attempting to manage. -type RBACRoleRule struct { - Groups RBACRoleRuleField - Resources RBACRoleRuleField - Verbs RBACRoleRuleField - URLs RBACRoleRuleField -} - -type RBACRoleRuleField []string - -type RBACRules []RBACRule - -func (r *RBACRule) addVerb(verb string) { - var found bool - - for _, existingVerb := range r.Verbs { - if existingVerb == verb { - found = true - - break - } - } - - if !found { - r.Verbs = append(r.Verbs, verb) - } -} - -func rbacGroupFromGroup(group string) string { - if group == "" { - return coreRBACGroup - } - - return group -} - -func rbacFieldsToString(verbs []string) string { - return strings.Join(verbs, ";") -} - -func (r *RBACRule) groupResourceEqual(newRBACRule *RBACRule) bool { - if r.Group == newRBACRule.Group && r.Resource == newRBACRule.Resource { - return true - } - - return false -} - -func (rs *RBACRules) groupResourceRecorded(newRBACRule *RBACRule) bool { - if rs == nil { - return false - } - - for _, r := range *rs { - r := r - if r.groupResourceEqual(newRBACRule) { - return true - } - } - - return false -} - -func (rs *RBACRules) hasURL(url string) bool { - for _, rule := range *rs { - if len(rule.URLs) == 0 { - continue - } - - for i := range rule.URLs { - if rule.URLs[i] == url { - return true - } - } - } - - return false -} - -func (r *RBACRule) ToMarker() string { - const kubebuilderPrefix = "// +kubebuilder:rbac" - - if len(r.URLs) > 0 { - return fmt.Sprintf("%s:verbs=%s,urls=%s", - kubebuilderPrefix, - rbacFieldsToString(r.Verbs), - rbacFieldsToString(r.URLs), - ) - } - - return fmt.Sprintf("%s:groups=%s,resources=%s,verbs=%s", - kubebuilderPrefix, - r.Group, - r.Resource, - rbacFieldsToString(r.Verbs), - ) -} - -func (rs *RBACRules) AddOrUpdateRules(newRules ...*RBACRule) { - for i := range newRules { - switch { - case newRules[i].hasGroupResource(): - rs.addForGroupResource(newRules[i]) - case newRules[i].hasURLs(): - rs.addForURLs(newRules[i]) - default: - continue - } - } -} - -func (rs *RBACRules) addForGroupResource(newRule *RBACRule) { - rules := *rs - - if !rules.groupResourceRecorded(newRule) { - *rs = append(*rs, *newRule) - } else { - for i := range rules { - if rules[i].groupResourceEqual(newRule) { - for _, verb := range newRule.Verbs { - rules[i].addVerb(verb) - } - } - } - } -} - -func (rs *RBACRules) addForURLs(newRule *RBACRule) { - rules := *rs - - for _, url := range newRule.URLs { - for i := range rules { - if rs.hasURL(url) { - for _, verb := range newRule.Verbs { - rules[i].addVerb(verb) - } - } else { - *rs = append(*rs, *newRule) - } - } - } -} - -func (r *RBACRule) hasGroupResource() bool { - return r.Group != "" && r.Resource != "" -} - -func (r *RBACRule) hasURLs() bool { - return len(r.URLs) > 0 -} - -func (rs *RBACRules) AddOrUpdateRoleRules(newRule *RBACRoleRule) { - // we must have verbs to create our rbac - if len(newRule.Verbs) == 0 { - return - } - - // we either need to have groups/resources or urls - if len(newRule.Groups) == 0 || len(newRule.Resources) == 0 { - if len(newRule.URLs) == 0 { - return - } - } - - // assign a new rule for each group and kind match - for _, rbacGroup := range newRule.Groups { - for _, rbacKind := range newRule.Resources { - rs.AddOrUpdateRules( - &RBACRule{ - Group: rbacGroupFromGroup(rbacGroup), - Resource: getResourceForRBAC(rbacKind), - Verbs: newRule.Verbs, - URLs: newRule.URLs, - }, - ) - } - } -} - -func getResourceForRBAC(kind string) string { - rbacResource := strings.Split(kind, "/") - - if rbacResource[0] == "*" { - kind = "*" - } else { - kind = getPluralRBAC(rbacResource[0]) - } - - if len(rbacResource) > 1 { - kind = fmt.Sprintf("%s/%s", kind, rbacResource[1]) - } - - return kind -} - -// getPluralRBAC will transform known irregulars into a proper type for rbac -// rules. -func getPluralRBAC(kind string) string { - pluralMap := map[string]string{ - "resourcequota": "resourcequotas", - } - plural := resource.RegularPlural(kind) - - if pluralMap[plural] != "" { - return pluralMap[plural] - } - - return plural -} - -func valueFromInterface(in interface{}, key string) (out interface{}) { - switch asType := in.(type) { - case map[interface{}]interface{}: - out = asType[key] - case map[string]interface{}: - out = asType[key] - case map[interface{}][]interface{}: - out = asType[key] - case map[string][]interface{}: - out = asType[key] - } - - return out -} - -func (rs *RBACRules) addRuleForWorkload(workload WorkloadAPIBuilder, forCollection bool) { - var verbs []string - - if forCollection { - verbs = []string{"get", "list", "watch"} - } else { - verbs = defaultResourceVerbs() - } - - // add permissions for the controller to be able to watch itself and update its own status - rs.AddOrUpdateRules( - &RBACRule{ - Group: fmt.Sprintf("%s.%s", workload.GetAPIGroup(), workload.GetDomain()), - Resource: getResourceForRBAC(workload.GetAPIKind()), - Verbs: verbs, - }, - &RBACRule{ - Group: fmt.Sprintf("%s.%s", workload.GetAPIGroup(), workload.GetDomain()), - Resource: fmt.Sprintf("%s/status", getResourceForRBAC(workload.GetAPIKind())), - Verbs: defaultStatusVerbs(), - }, - ) -} - -func (rs *RBACRules) addRulesForWorkload(workload WorkloadAPIBuilder) { - rs.addRuleForWorkload(workload, false) - - if workload.IsComponent() { - rs.addRuleForWorkload(workload.GetCollection(), true) - } -} - -func (rs *RBACRules) addRulesForManifest(kind, group string, rawContent interface{}) error { - rs.AddOrUpdateRules( - &RBACRule{ - Group: group, - Resource: getResourceForRBAC(kind), - Verbs: defaultResourceVerbs(), - }, - ) - - // if we are working with roles and cluster roles, we must also grant rbac to the resources - // which are managed by them - if strings.EqualFold(kind, "clusterrole") || strings.EqualFold(kind, "role") { - rules := valueFromInterface(rawContent, "rules") - if rules == nil { - return nil - } - - rbacRoleRules, err := toArrayInterface(rules) - if err != nil { - return fmt.Errorf("%w; error converting resource rules %v", err, rules) - } - - for _, rbacRoleRule := range rbacRoleRules { - rule := &RBACRoleRule{} - if err := rule.processRawRoleRule(rbacRoleRule); err != nil { - return fmt.Errorf("%w; error processing rbac role rule %v", err, rbacRoleRule) - } - - rs.AddOrUpdateRoleRules(rule) - } - } - - return nil -} - -func (roleRule *RBACRoleRule) processRawRoleRule(rule interface{}) error { - fields := map[*RBACRoleRuleField]string{ - &roleRule.Groups: "apiGroups", - &roleRule.Resources: "resources", - &roleRule.Verbs: "verbs", - &roleRule.URLs: "nonResourceURLs", - } - - for objectField, fieldKey := range fields { - if err := objectField.setRbacRoleRuleField(rule, fieldKey); err != nil { - return fmt.Errorf("%w; error processing raw fule %v", err, rule) - } - } - - return nil -} - -func (field *RBACRoleRuleField) setRbacRoleRuleField(rule interface{}, fieldKey string) error { - fieldValue := valueFromInterface(rule, fieldKey) - if fieldValue == nil { - return nil - } - - fieldValues, err := toArrayString(fieldValue) - if err != nil { - return fmt.Errorf("%w; error converting rbac field key %s for rule %v", err, fieldKey, rule) - } - - *field = fieldValues - - return nil -} - -func defaultStatusVerbs() []string { - return []string{ - "get", "update", "patch", - } -} - -func defaultResourceVerbs() []string { - return []string{ - "get", "list", "watch", "create", "update", "patch", "delete", - } -} diff --git a/internal/workload/v1/rbac/rbac.go b/internal/workload/v1/rbac/rbac.go new file mode 100644 index 0000000..93821a3 --- /dev/null +++ b/internal/workload/v1/rbac/rbac.go @@ -0,0 +1,149 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: MIT + +package rbac + +import ( + "fmt" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "sigs.k8s.io/kubebuilder/v3/pkg/model/resource" +) + +// rbacWorkloadProcessor is an interface which implements processing of rbac rules +// for individual workloads (e.g. standalone, collection, component). +type rbacWorkloadProcessor interface { + IsComponent() bool + + GetDomain() string + GetAPIGroup() string + GetAPIVersion() string + GetAPIKind() string +} + +// rbacRuleProcessor is an interface which implements processing of individual +// rbac rules. +type rbacRuleProcessor interface { + addTo(*Rules) +} + +const ( + coreGroup = "core" + kubebuilderPrefix = "// +kubebuilder:rbac" +) + +// defaultResourceVerbs is a helper function to define the default verbs that are allowed +// for resources that are managed by the scaffolded controller. +func defaultResourceVerbs() []string { + return []string{ + "get", "list", "watch", "create", "update", "patch", "delete", + } +} + +// defaultStatusVerbs is a helper function to define the default verbs which get placed +// onto resources that have a /status suffix. +func defaultStatusVerbs() []string { + return []string{ + "get", "update", "patch", + } +} + +// knownIrregulars is a helper function to define known irregular kinds and their +// expected formats. +// - keys = found values +// - values = proper values +func knownIrregulars() map[string]string { + return map[string]string{ + "resourcequota": "resourcequotas", + } +} + +// ForManifest will return a set of rules for a particular manifest. This includes +// a rule for the manifest itself, in addition to adding particular rules for whatever +// roles and cluster roles are requesting. This is because the controller needs to have +// permissions to manage the children that roles and cluster roles are requesting. +func ForManifest(manifest *unstructured.Unstructured) (*Rules, error) { + rules := &Rules{} + + if err := rules.addForManifest(manifest); err != nil { + return rules, err + } + + return rules, nil +} + +// ForWorkloads will return a set of rules for a particular set of workloads. It should be noted that +// this only returns the specific rules for the actual workload and not the managed resources. See +// ForManifest for details on the rules for a particular manifest. +func ForWorkloads(workloads ...rbacWorkloadProcessor) *Rules { + rules := &Rules{} + + // for each of the workloads passed in, add a rule to the set of rules + for i := range workloads { + rules.addForWorkload(workloads[i]) + } + + return rules +} + +// getGroup returns the group in the proper format as expected by rbac markers. +func getGroup(group string) string { + if group == "" { + return coreGroup + } + + return group +} + +// getFieldString returns an array of fields in string format. +func getFieldString(fields []string) string { + return strings.Join(fields, ";") +} + +// getResource gets the resource properly formatted for an rbac rule given the kind +// of resource. For regular rules, the kind comes in as expected, but for role +// rules, this could come in as an asterisk so it has to be specially handled. +func getResource(kind string) string { + rbacResource := strings.Split(kind, "/") + + if rbacResource[0] == "*" { + kind = "*" + } else { + kind = getPlural(rbacResource[0]) + } + + if len(rbacResource) > 1 { + kind = fmt.Sprintf("%s/%s", kind, rbacResource[1]) + } + + return kind +} + +// getPlural will transform known irregulars into a proper type for rbac +// rules. +func getPlural(kind string) string { + plural := resource.RegularPlural(kind) + + if knownIrregulars()[plural] != "" { + return knownIrregulars()[plural] + } + + return plural +} + +// valueFromInterface attempts to retrieve a value from an interface as a map. +func valueFromInterface(in interface{}, key string) (out interface{}) { + switch asType := in.(type) { + case map[interface{}]interface{}: + out = asType[key] + case map[string]interface{}: + out = asType[key] + case map[interface{}][]interface{}: + out = asType[key] + case map[string][]interface{}: + out = asType[key] + } + + return out +} diff --git a/internal/workload/v1/rbac/rbac_internal_test.go b/internal/workload/v1/rbac/rbac_internal_test.go new file mode 100644 index 0000000..b0eefb0 --- /dev/null +++ b/internal/workload/v1/rbac/rbac_internal_test.go @@ -0,0 +1,262 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: MIT + +package rbac + +import ( + "reflect" + "testing" +) + +func Test_getGroup(t *testing.T) { + t.Parallel() + + type args struct { + group string + } + + tests := []struct { + name string + args args + want string + }{ + { + name: "ensure empty group returns core group", + args: args{ + group: "", + }, + want: coreGroup, + }, + { + name: "ensure other group returns itself", + args: args{ + group: "thisisatestgroup", + }, + want: "thisisatestgroup", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := getGroup(tt.args.group); got != tt.want { + t.Errorf("getGroup() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_getFieldString(t *testing.T) { + t.Parallel() + + type args struct { + fields []string + } + + tests := []struct { + name string + args args + want string + }{ + { + name: "ensure strings return semicolon separated string", + args: args{ + fields: []string{"one", "two", "three"}, + }, + want: "one;two;three", + }, + { + name: "ensure empty string array returns empty string", + args: args{ + fields: []string{}, + }, + want: "", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := getFieldString(tt.args.fields); got != tt.want { + t.Errorf("getFieldString() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_getResource(t *testing.T) { + t.Parallel() + + type args struct { + kind string + } + + tests := []struct { + name string + args args + want string + }{ + { + name: "ensure status kind returns the plural kind with the status appended", + args: args{ + kind: "apple/status", + }, + want: "apples/status", + }, + { + name: "ensure wildcard kind returns wildcard", + args: args{ + kind: "*", + }, + want: "*", + }, + { + name: "ensure wildcard kind with slash returns wildcard with slash", + args: args{ + kind: "*/status", + }, + want: "*/status", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := getResource(tt.args.kind); got != tt.want { + t.Errorf("getResource() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_getPlural(t *testing.T) { + t.Parallel() + + type args struct { + kind string + } + + tests := []struct { + name string + args args + want string + }{ + { + name: "ensure kind returns correct plural value", + args: args{ + kind: "apples", + }, + want: "apples", + }, + { + name: "ensure kind with an irregular plural returns correct plural value", + args: args{ + kind: "resourcequota", + }, + want: "resourcequotas", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := getPlural(tt.args.kind); got != tt.want { + t.Errorf("getPlural() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_valueFromInterface(t *testing.T) { + t.Parallel() + + mapIntInt := map[interface{}]interface{}{ + "key": "value", + } + + mapStringInt := map[string]interface{}{ + "key": "value", + } + + mapIntArrayInt := map[interface{}][]interface{}{ + "key": {"value"}, + } + + mapStringArrayInt := map[string][]interface{}{ + "key": {"value"}, + } + + type args struct { + in interface{} + key string + } + + tests := []struct { + name string + args args + wantOut interface{} + }{ + { + name: "ensure map interface interface returns value", + args: args{ + in: mapIntInt, + key: "key", + }, + wantOut: "value", + }, + { + name: "ensure map string interface returns value", + args: args{ + in: mapStringInt, + key: "key", + }, + wantOut: "value", + }, + { + name: "ensure map interface array interface returns value", + args: args{ + in: mapIntArrayInt, + key: "key", + }, + wantOut: []interface{}{"value"}, + }, + { + name: "ensure map string array interface returns value", + args: args{ + in: mapStringArrayInt, + key: "key", + }, + wantOut: []interface{}{"value"}, + }, + { + name: "ensure unknown returns nil", + args: args{ + in: []string{"test"}, + key: "key", + }, + wantOut: nil, + }, + { + name: "ensure nil returns nil", + args: args{ + in: nil, + key: "key", + }, + wantOut: nil, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if gotOut := valueFromInterface(tt.args.in, tt.args.key); !reflect.DeepEqual(gotOut, tt.wantOut) { + t.Errorf("valueFromInterface() = %v, want %v", gotOut, tt.wantOut) + } + }) + } +} diff --git a/internal/workload/v1/rbac/role_rule.go b/internal/workload/v1/rbac/role_rule.go new file mode 100644 index 0000000..d833a8e --- /dev/null +++ b/internal/workload/v1/rbac/role_rule.go @@ -0,0 +1,125 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: MIT + +package rbac + +import ( + "errors" + "fmt" + + "github.com/vmware-tanzu-labs/operator-builder/internal/utils" +) + +var ( + ErrorProcessRoleRule = errors.New("error processing role rule") + ErrorProcessRoleRuleField = errors.New("error processing role rule field") +) + +// RoleRule contains the info needed to create the kubebuilder:rbac markers +// in the controller when a resource that is of a role or clusterrole type is +// found. This is because the underlying controller needs the same permissions +// for the role or clusterrole that it is attempting to manage. +type RoleRule struct { + Groups RoleRuleField + Resources RoleRuleField + Verbs RoleRuleField + URLs RoleRuleField +} + +type RoleRuleField []string + +// addTo satisfies the rbacRuleProcessor interface by defining the logic that adds a +// role rule into an existing set of rules. +func (roleRule *RoleRule) addTo(rules *Rules) { + // convert the role rule into a set of rules + newRules := roleRule.toRules() + + for _, rule := range *newRules { + rule.addTo(rules) + } +} + +// processRaw will take in a raw interface and convert it into a role rule. +func (roleRule *RoleRule) processRaw(rule interface{}) error { + fields := map[*RoleRuleField]string{ + &roleRule.Groups: "apiGroups", + &roleRule.Resources: "resources", + &roleRule.Verbs: "verbs", + &roleRule.URLs: "nonResourceURLs", + } + + for objectField, fieldKey := range fields { + if err := objectField.setValues(rule, fieldKey); err != nil { + return fmt.Errorf("%w; %s: %v", err, ErrorProcessRoleRule, rule) + } + } + + return nil +} + +// setValues of a field for a particular role rule. +func (field *RoleRuleField) setValues(rule interface{}, fieldKey string) error { + fieldValue := valueFromInterface(rule, fieldKey) + if fieldValue == nil { + return nil + } + + fieldValues, err := utils.ToArrayString(fieldValue) + if err != nil { + return fmt.Errorf("%w; %s: [%s]", err, ErrorProcessRoleRuleField, fieldKey) + } + + *field = fieldValues + + return nil +} + +// toRules will convert a role rule into a set of regular rules. +func (roleRule *RoleRule) toRules() *Rules { + // we must have verbs to create our rbac + if len(roleRule.Verbs) == 0 { + return &Rules{} + } + + // we either need to have groups/resources or urls + if len(roleRule.Groups) > 0 && len(roleRule.Resources) > 0 { + return roleRule.groupResourceRules() + } else if len(roleRule.URLs) > 0 { + return roleRule.nonResourceRules() + } + + return &Rules{} +} + +// groupResourceRules will return a set of rules given a role rule which contains +// both a group and a resource. +func (roleRule *RoleRule) groupResourceRules() *Rules { + rules := &Rules{} + + // assign a new rule for each group and kind match + for _, rbacGroup := range roleRule.Groups { + for _, rbacKind := range roleRule.Resources { + rule := &Rule{ + Group: getGroup(rbacGroup), + Resource: getResource(rbacKind), + Verbs: roleRule.Verbs, + URLs: roleRule.URLs, + } + + rule.addResourceRuleTo(rules) + } + } + + return rules +} + +// nonResourceRules will return a set of rules given a role rule which does not +// contain a group and a resource and instead contains non resource urls. +func (roleRule *RoleRule) nonResourceRules() *Rules { + return &Rules{ + { + Verbs: roleRule.Verbs, + URLs: roleRule.URLs, + }, + } +} diff --git a/internal/workload/v1/rbac/role_rule_internal_test.go b/internal/workload/v1/rbac/role_rule_internal_test.go new file mode 100644 index 0000000..24a8437 --- /dev/null +++ b/internal/workload/v1/rbac/role_rule_internal_test.go @@ -0,0 +1,359 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: MIT + +package rbac + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" +) + +func NewTestRoleRule() *RoleRule { + return &RoleRule{ + Groups: []string{"apps"}, + Resources: []string{"DaemonSets", "Deployments"}, + Verbs: []string{"get", "patch"}, + } +} + +func TestRoleRule_addTo(t *testing.T) { + t.Parallel() + + testRule := NewTestRule() + + type fields struct { + Groups []string + Resources []string + URLs []string + Verbs []string + } + + type args struct { + rules *Rules + } + + tests := []struct { + name string + fields fields + args args + want *Rules + }{ + { + name: "ensure new role rule is added properly", + fields: fields{ + Groups: []string{"newGroup"}, + Resources: []string{"newResources"}, + Verbs: []string{"test"}, + }, + args: args{ + rules: &Rules{}, + }, + want: &Rules{ + { + Group: "newGroup", + Resource: "newresources", + Verbs: []string{"test"}, + }, + }, + }, + { + name: "ensure existing rule is not added", + fields: fields{ + Groups: []string{testRule.Group}, + Resources: []string{testRule.Resource}, + Verbs: testRule.Verbs, + }, + args: args{ + rules: NewTestRules(), + }, + want: NewTestRules(), + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rule := &RoleRule{ + Groups: tt.fields.Groups, + Resources: tt.fields.Resources, + URLs: tt.fields.URLs, + Verbs: tt.fields.Verbs, + } + rule.addTo(tt.args.rules) + assert.Equal(t, tt.want, tt.args.rules) + }) + } +} + +func TestRoleRuleField_setValues(t *testing.T) { + t.Parallel() + + rule := map[string]interface{}{ + "apiGroups": []string{ + "one", + "two", + "three", + }, + } + + invalid := map[string]interface{}{ + "is": "invalid", + } + + type args struct { + rule interface{} + fieldKey string + } + + tests := []struct { + name string + field *RoleRuleField + args args + wantErr bool + want *RoleRuleField + }{ + { + name: "ensure field is set appropriately without an error", + field: &RoleRuleField{}, + args: args{ + rule: rule, + fieldKey: "apiGroups", + }, + wantErr: false, + want: &RoleRuleField{ + "one", + "two", + "three", + }, + }, + { + name: "ensure missing key returns no error with no changes", + field: &RoleRuleField{}, + args: args{ + rule: rule, + fieldKey: "missing", + }, + wantErr: false, + want: &RoleRuleField{}, + }, + { + name: "ensure invalid rule returns an error with no changes", + field: &RoleRuleField{}, + args: args{ + rule: invalid, + fieldKey: "is", + }, + wantErr: true, + want: &RoleRuleField{}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if err := tt.field.setValues(tt.args.rule, tt.args.fieldKey); (err != nil) != tt.wantErr { + t.Errorf("RoleRuleField.setValues() error = %v, wantErr %v", err, tt.wantErr) + } + assert.Equal(t, tt.want, tt.field) + }) + } +} + +func TestRoleRule_processRaw(t *testing.T) { + t.Parallel() + + rule := map[string][]interface{}{ + "apiGroups": { + "dog", + "cat", + }, + "resources": { + "toy", + "treat", + }, + "verbs": { + "bark", + "meow", + }, + "nonResourceURLs": { + "/dog", + "/cat", + }, + } + + invalid := map[string]interface{}{ + "apiGroups": "group", + } + + type fields struct { + Groups RoleRuleField + Resources RoleRuleField + Verbs RoleRuleField + URLs RoleRuleField + } + + type args struct { + rule interface{} + } + + tests := []struct { + name string + fields fields + args args + wantErr bool + want *RoleRule + }{ + { + name: "ensure valid rule is set appropriately without error", + args: args{ + rule: rule, + }, + wantErr: false, + want: &RoleRule{ + Groups: RoleRuleField{ + "dog", + "cat", + }, + Resources: RoleRuleField{ + "toy", + "treat", + }, + Verbs: RoleRuleField{ + "bark", + "meow", + }, + URLs: RoleRuleField{ + "/dog", + "/cat", + }, + }, + }, + { + name: "ensure invalid rule returns error and does not change role rule", + args: args{ + rule: invalid, + }, + wantErr: true, + want: &RoleRule{}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + roleRule := &RoleRule{ + Groups: tt.fields.Groups, + Resources: tt.fields.Resources, + Verbs: tt.fields.Verbs, + URLs: tt.fields.URLs, + } + if err := roleRule.processRaw(tt.args.rule); (err != nil) != tt.wantErr { + t.Errorf("RoleRule.processRaw() error = %v, wantErr %v", err, tt.wantErr) + } + assert.Equal(t, tt.want, roleRule) + }) + } +} + +func TestRoleRule_toRules(t *testing.T) { + t.Parallel() + + resourceRoleRule := &RoleRule{ + Groups: RoleRuleField{ + "core", + }, + Resources: RoleRuleField{ + "dogs", + }, + Verbs: RoleRuleField{ + "pet", + }, + } + + nonResourceRoleRule := &RoleRule{ + URLs: RoleRuleField{ + "/bark", + }, + Verbs: RoleRuleField{ + "pet", + }, + } + + type fields struct { + Groups RoleRuleField + Resources RoleRuleField + Verbs RoleRuleField + URLs RoleRuleField + } + + tests := []struct { + name string + fields fields + wantRules *Rules + }{ + { + name: "role rule without verbs returns empty rules", + fields: fields{ + Verbs: RoleRuleField{}, + }, + wantRules: &Rules{}, + }, + { + name: "invalid role rule returns empty rules", + fields: fields{ + Groups: resourceRoleRule.Groups, + Verbs: resourceRoleRule.Verbs, + }, + wantRules: &Rules{}, + }, + { + name: "resource role rule returns resource rule", + fields: fields{ + Groups: resourceRoleRule.Groups, + Resources: resourceRoleRule.Resources, + Verbs: resourceRoleRule.Verbs, + }, + wantRules: &Rules{ + { + Group: "core", + Resource: "dogs", + Verbs: []string{"pet"}, + }, + }, + }, + { + name: "non-resource role rule returns non-resource rule", + fields: fields{ + URLs: nonResourceRoleRule.URLs, + Verbs: nonResourceRoleRule.Verbs, + }, + wantRules: &Rules{ + { + URLs: []string{"/bark"}, + Verbs: []string{"pet"}, + }, + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + roleRule := &RoleRule{ + Groups: tt.fields.Groups, + Resources: tt.fields.Resources, + Verbs: tt.fields.Verbs, + URLs: tt.fields.URLs, + } + if gotRules := roleRule.toRules(); !reflect.DeepEqual(gotRules, tt.wantRules) { + t.Errorf("RoleRule.toRules() = %v, want %v", gotRules, tt.wantRules) + } + }) + } +} diff --git a/internal/workload/v1/rbac/rule.go b/internal/workload/v1/rbac/rule.go new file mode 100644 index 0000000..5f02aa3 --- /dev/null +++ b/internal/workload/v1/rbac/rule.go @@ -0,0 +1,135 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: MIT + +package rbac + +import ( + "fmt" +) + +// Rule contains the info needed to create the kubebuilder:rbac markers in +// the controller. +type Rule struct { + Group string + Resource string + URLs []string + Verbs []string +} + +// ToMarker will return a specific marker in string format. +func (rule *Rule) ToMarker() string { + if len(rule.URLs) > 0 { + return fmt.Sprintf("%s:verbs=%s,urls=%s", + kubebuilderPrefix, + getFieldString(rule.Verbs), + getFieldString(rule.URLs), + ) + } + + return fmt.Sprintf("%s:groups=%s,resources=%s,verbs=%s", + kubebuilderPrefix, + rule.Group, + rule.Resource, + getFieldString(rule.Verbs), + ) +} + +// addTo satisfies the rbacRuleProcessor interface by defining the logic that adds a rule into an +// existing set of rules. +func (rule *Rule) addTo(rules *Rules) { + if len(*rules) == 0 { + *rules = append(*rules, *rule) + + return + } + + if rule.isResourceRule() { + rule.addResourceRuleTo(rules) + } else { + rule.addNonResourceRuleTo(rules) + } +} + +// addResourceRuleTo will add a resource rule to a set of rules. +func (rule *Rule) addResourceRuleTo(rules *Rules) { + rs := *rules + + if !rules.hasResourceRule(rule) { + *rules = append(*rules, *rule) + } else { + for i := range rs { + existingRule := &rs[i] + + if rule.groupResourceEqual(existingRule) { + for _, verb := range rule.Verbs { + rs[i].addVerb(verb) + } + } + } + } +} + +// addNonResourceRuleTo will add a non-resource rule to a set of rules. +func (rule *Rule) addNonResourceRuleTo(rules *Rules) { + for _, url := range rule.URLs { + for _, existingRule := range *rules { + if existingRule.hasURL(url) { + for _, verb := range rule.Verbs { + existingRule.addVerb(verb) + } + + return + } + } + } + + *rules = append(*rules, *rule) +} + +// addVerb adds a verb to an existing rule. The verb is only added if it is not +// found to prevent duplication of markers that are generated in the controller. +func (rule *Rule) addVerb(verb string) { + var found bool + + for _, existingVerb := range rule.Verbs { + if existingVerb == verb { + found = true + + break + } + } + + if !found { + rule.Verbs = append(rule.Verbs, verb) + } +} + +// groupResourceEqual determines if the group and resource are equal given an +// input rule. +func (rule *Rule) groupResourceEqual(compared *Rule) bool { + if rule.Group == compared.Group && rule.Resource == compared.Resource { + return true + } + + return false +} + +// isResourceRule determines if a rule is a resource rule or not. +func (rule *Rule) isResourceRule() bool { + if rule.Group != "" && rule.Resource != "" { + return true + } + + return false +} + +// hasURL determines if a set of rules contains a url. +func (rule *Rule) hasURL(url string) bool { + for i := range rule.URLs { + if rule.URLs[i] == url { + return true + } + } + + return false +} diff --git a/internal/workload/v1/rbac/rule_internal_test.go b/internal/workload/v1/rbac/rule_internal_test.go new file mode 100644 index 0000000..ea260e2 --- /dev/null +++ b/internal/workload/v1/rbac/rule_internal_test.go @@ -0,0 +1,577 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: MIT + +package rbac + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func NewTestRule() *Rule { + return &Rule{ + Group: "core", + Resource: "exampleresources", + Verbs: []string{"get", "patch"}, + } +} + +func NewTestNonResourceRule() *Rule { + return &Rule{ + URLs: []string{"/metrics"}, + Verbs: []string{"get", "patch"}, + } +} + +func NewTestRules() *Rules { + testRule := NewTestRule() + testNonResourceRule := NewTestNonResourceRule() + testRules := Rules{} + testRules = append(testRules, *testRule, *testNonResourceRule) + + return &testRules +} + +func TestRule_ToMarker(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + rule *Rule + want string + }{ + { + name: "ensure resource rbac marker returns as expected", + rule: NewTestRule(), + want: "// +kubebuilder:rbac:groups=core,resources=exampleresources,verbs=get;patch", + }, + { + name: "ensure non-resource rbac marker returns as expected", + rule: NewTestNonResourceRule(), + want: "// +kubebuilder:rbac:verbs=get;patch,urls=/metrics", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rule := tt.rule + if got := rule.ToMarker(); got != tt.want { + t.Errorf("Rule.ToMarker() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestRule_addTo(t *testing.T) { + t.Parallel() + + testRule := NewTestRule() + testNonResourceRule := NewTestNonResourceRule() + + type fields struct { + Group string + Resource string + URLs []string + Verbs []string + } + + type args struct { + rules *Rules + } + + tests := []struct { + name string + fields fields + args args + want *Rules + }{ + { + name: "ensure new rule is added properly", + fields: fields{ + Group: "newGroup", + Resource: "newResource", + Verbs: []string{"test"}, + }, + args: args{ + rules: &Rules{}, + }, + want: &Rules{ + { + Group: "newGroup", + Resource: "newResource", + Verbs: []string{"test"}, + }, + }, + }, + { + name: "ensure new non-resource rule is added properly", + fields: fields{ + URLs: []string{"yes"}, + Verbs: []string{"test"}, + }, + args: args{ + rules: &Rules{}, + }, + want: &Rules{ + { + URLs: []string{"yes"}, + Verbs: []string{"test"}, + }, + }, + }, + { + name: "ensure existing rule is not added", + fields: fields{ + Group: testRule.Group, + Resource: testRule.Resource, + Verbs: testRule.Verbs, + }, + args: args{ + rules: NewTestRules(), + }, + want: NewTestRules(), + }, + { + name: "ensure existing non-resource rule is not added", + fields: fields{ + URLs: testNonResourceRule.URLs, + Verbs: testNonResourceRule.Verbs, + }, + args: args{ + rules: NewTestRules(), + }, + want: NewTestRules(), + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rule := &Rule{ + Group: tt.fields.Group, + Resource: tt.fields.Resource, + URLs: tt.fields.URLs, + Verbs: tt.fields.Verbs, + } + rule.addTo(tt.args.rules) + assert.Equal(t, tt.want, tt.args.rules) + }) + } +} + +func TestRule_addResourceRuleTo(t *testing.T) { + t.Parallel() + + testRule := NewTestRule() + + type fields struct { + Group string + Resource string + URLs []string + Verbs []string + } + + type args struct { + rules *Rules + } + + tests := []struct { + name string + fields fields + args args + want *Rules + }{ + { + name: "ensure new rule is added properly", + fields: fields{ + Group: "newGroup", + Resource: "newResource", + Verbs: []string{"test"}, + }, + args: args{ + rules: &Rules{}, + }, + want: &Rules{ + { + Group: "newGroup", + Resource: "newResource", + Verbs: []string{"test"}, + }, + }, + }, + { + name: "ensure new rule with rule is added properly", + fields: fields{ + Group: "newGroup", + Resource: "newResource", + Verbs: []string{"test"}, + }, + args: args{ + rules: &Rules{ + *NewTestRule(), + }, + }, + want: &Rules{ + *NewTestRule(), + { + Group: "newGroup", + Resource: "newResource", + Verbs: []string{"test"}, + }, + }, + }, + { + name: "ensure existing rule is not added", + fields: fields{ + Group: testRule.Group, + Resource: testRule.Resource, + Verbs: testRule.Verbs, + }, + args: args{ + rules: NewTestRules(), + }, + want: NewTestRules(), + }, + { + name: "ensure existing rule with new verbs are appended appropriately", + fields: fields{ + Group: testRule.Group, + Resource: testRule.Resource, + Verbs: []string{"existing"}, + }, + args: args{ + &Rules{ + { + Group: testRule.Group, + Resource: testRule.Resource, + Verbs: testRule.Verbs, + }, + }, + }, + want: &Rules{ + { + Group: testRule.Group, + Resource: testRule.Resource, + Verbs: []string{"get", "patch", "existing"}, + }, + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rule := &Rule{ + Group: tt.fields.Group, + Resource: tt.fields.Resource, + URLs: tt.fields.URLs, + Verbs: tt.fields.Verbs, + } + rule.addResourceRuleTo(tt.args.rules) + assert.Equal(t, tt.want, tt.args.rules) + }) + } +} + +func TestRule_addNonResourceRuleTo(t *testing.T) { + t.Parallel() + + testNonResourceRule := NewTestNonResourceRule() + + type fields struct { + Group string + Resource string + URLs []string + Verbs []string + } + + type args struct { + rules *Rules + } + + tests := []struct { + name string + fields fields + args args + want *Rules + }{ + { + name: "ensure new non-resource rule is added properly", + fields: fields{ + URLs: []string{"yes"}, + Verbs: []string{"test"}, + }, + args: args{ + rules: &Rules{}, + }, + want: &Rules{ + { + URLs: []string{"yes"}, + Verbs: []string{"test"}, + }, + }, + }, + { + name: "ensure new non-resource rule with rule is added properly", + fields: fields{ + URLs: []string{"/existing"}, + Verbs: []string{"test"}, + }, + args: args{ + rules: &Rules{ + *NewTestRule(), + }, + }, + want: &Rules{ + *NewTestRule(), + { + URLs: []string{"/existing"}, + Verbs: []string{"test"}, + }, + }, + }, + { + name: "ensure existing non-resource rule is not added", + fields: fields{ + URLs: testNonResourceRule.URLs, + Verbs: testNonResourceRule.Verbs, + }, + args: args{ + rules: NewTestRules(), + }, + want: NewTestRules(), + }, + { + name: "ensure existing non-resulte rule with new urls are appended appropriately", + fields: fields{ + URLs: []string{"/new"}, + Verbs: testNonResourceRule.Verbs, + }, + args: args{ + &Rules{ + { + URLs: testNonResourceRule.URLs, + Verbs: testNonResourceRule.Verbs, + }, + }, + }, + want: &Rules{ + { + URLs: []string{"/metrics"}, + Verbs: testNonResourceRule.Verbs, + }, + { + URLs: []string{"/new"}, + Verbs: testNonResourceRule.Verbs, + }, + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rule := &Rule{ + Group: tt.fields.Group, + Resource: tt.fields.Resource, + URLs: tt.fields.URLs, + Verbs: tt.fields.Verbs, + } + rule.addNonResourceRuleTo(tt.args.rules) + assert.Equal(t, tt.want, tt.args.rules) + }) + } +} + +func TestRule_addVerb(t *testing.T) { + t.Parallel() + + type args struct { + verb string + } + + tests := []struct { + name string + rbac *Rule + args args + want []string + }{ + { + name: "ensure new verb is appropriately added", + rbac: NewTestRule(), + args: args{ + verb: "delete", + }, + want: []string{"get", "patch", "delete"}, + }, + { + name: "ensure existing verb is not added", + rbac: NewTestRule(), + args: args{ + verb: "get", + }, + want: []string{"get", "patch"}, + }, + { + name: "ensure new verb is appropriately added to a non-resource rule", + rbac: NewTestNonResourceRule(), + args: args{ + verb: "put", + }, + want: []string{"get", "patch", "put"}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + r := tt.rbac + r.addVerb(tt.args.verb) + + assert.Equal(t, r.Verbs, tt.want) + }) + } +} + +func TestRule_groupResourceEqual(t *testing.T) { + t.Parallel() + + type fields struct { + Group string + Resource string + URLs []string + Verbs []string + } + + type args struct { + compared *Rule + } + + tests := []struct { + name string + fields fields + args args + want bool + }{ + { + name: "ensure rule with equal group and resource returns true", + fields: fields{ + Group: "core", + Resource: "exampleresources", + }, + args: args{ + compared: NewTestRule(), + }, + want: true, + }, + { + name: "ensure rule with not equal group and resource returns false", + fields: fields{ + Group: "coreFake", + Resource: "exampleResourceFake", + }, + args: args{ + compared: NewTestRule(), + }, + want: false, + }, + { + name: "ensure rule with not equal group returns false", + fields: fields{ + Group: "coreFake", + Resource: "exampleresources", + }, + args: args{ + compared: NewTestRule(), + }, + want: false, + }, + { + name: "ensure rule with not equal resource returns false", + fields: fields{ + Group: "core", + Resource: "exampleResourceFake", + }, + args: args{ + compared: NewTestRule(), + }, + want: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rule := &Rule{ + Group: tt.fields.Group, + Resource: tt.fields.Resource, + URLs: tt.fields.URLs, + Verbs: tt.fields.Verbs, + } + if got := rule.groupResourceEqual(tt.args.compared); got != tt.want { + t.Errorf("Rule.groupResourceEqual() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestRule_isResourceRule(t *testing.T) { + t.Parallel() + + testRule := NewTestRule() + testNonResourceRule := NewTestNonResourceRule() + + type fields struct { + Group string + Resource string + URLs []string + Verbs []string + } + + tests := []struct { + name string + fields fields + want bool + }{ + { + name: "ensure non resource rule returns false", + fields: fields{ + URLs: testNonResourceRule.URLs, + Verbs: testNonResourceRule.Verbs, + }, + want: false, + }, + { + name: "ensure non resource rule returns false", + fields: fields{ + Group: testRule.Group, + Resource: testRule.Resource, + }, + want: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + rule := &Rule{ + Group: tt.fields.Group, + Resource: tt.fields.Resource, + URLs: tt.fields.URLs, + Verbs: tt.fields.Verbs, + } + if got := rule.isResourceRule(); got != tt.want { + t.Errorf("Rule.isResourceRule() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/workload/v1/rbac/rules.go b/internal/workload/v1/rbac/rules.go new file mode 100644 index 0000000..7fa71b7 --- /dev/null +++ b/internal/workload/v1/rbac/rules.go @@ -0,0 +1,106 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: MIT + +package rbac + +import ( + "fmt" + "strings" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + "github.com/vmware-tanzu-labs/operator-builder/internal/utils" +) + +type Rules []Rule + +// add will add a set of new rules to an existing set of rules. +func (rules *Rules) Add(newRules ...rbacRuleProcessor) { + for i := range newRules { + newRules[i].addTo(rules) + } +} + +// addTo satisfies the rbacRuleProcessor interface by defining the logic that adds a rule into an +// existing set of rules. +func (rules *Rules) addTo(ruleSet *Rules) { + rs := *rules + + for i := range *rules { + rule := rs[i] + + ruleSet.Add(&rule) + } +} + +// addForWorkload will add a particular rule to a set of rules given a workload. +func (rules *Rules) addForWorkload(workload rbacWorkloadProcessor) { + // workloadRule is a rule that creates rbac such that the controller can manage the + // workload type in which it is responsible for reconciling + workloadRule := &Rule{ + Group: fmt.Sprintf("%s.%s", workload.GetAPIGroup(), workload.GetDomain()), + Resource: getResource(workload.GetAPIKind()), + Verbs: defaultResourceVerbs(), + } + + // statusRule is a rule that creates rbac such that the controller can manage its own + // status updates for the workload that it is responsible for reconciling + statusRule := &Rule{ + Group: fmt.Sprintf("%s.%s", workload.GetAPIGroup(), workload.GetDomain()), + Resource: fmt.Sprintf("%s/status", getResource(workload.GetAPIKind())), + Verbs: defaultStatusVerbs(), + } + + rules.Add(workloadRule, statusRule) +} + +// addForManifest will add a particular rule given an unstructured manifest. +func (rules *Rules) addForManifest(manifest *unstructured.Unstructured) error { + kind := manifest.GetKind() + + rules.Add( + &Rule{ + Group: getGroup(manifest.GroupVersionKind().Group), + Resource: getResource(kind), + Verbs: defaultResourceVerbs(), + }, + ) + + // if we are working with roles and cluster roles, we must also grant rbac to the resources + // which are managed by them + if strings.EqualFold(kind, "clusterrole") || strings.EqualFold(kind, "role") { + roleRules := valueFromInterface(manifest.Object, "rules") + if roleRules == nil { + return nil + } + + rbacRoleRules, err := utils.ToArrayInterface(roleRules) + if err != nil { + return fmt.Errorf("%w; error converting resource rules %v", err, roleRules) + } + + for _, rbacRoleRule := range rbacRoleRules { + rule := &RoleRule{} + if err := rule.processRaw(rbacRoleRule); err != nil { + return fmt.Errorf("%w; error processing rbac role rule %v", err, rbacRoleRule) + } + + rules.Add(rule) + } + } + + return nil +} + +// hasResourceRule determines if a set of rules has a rule which contains +// a specific group/resource combination. A specific group/resource combination +// is used to guarantee uniqueness on a set of rules. +func (rules *Rules) hasResourceRule(rule *Rule) bool { + for _, r := range *rules { + if r.groupResourceEqual(rule) { + return true + } + } + + return false +} diff --git a/internal/workload/v1/rbac/rules_internal_test.go b/internal/workload/v1/rbac/rules_internal_test.go new file mode 100644 index 0000000..4777c5c --- /dev/null +++ b/internal/workload/v1/rbac/rules_internal_test.go @@ -0,0 +1,293 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: MIT + +package rbac + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" +) + +func TestRules_hasResourceRule(t *testing.T) { + t.Parallel() + + type args struct { + rule *Rule + } + + tests := []struct { + name string + rules *Rules + args args + want bool + }{ + { + name: "ensure rule set with existing rule returns true", + rules: NewTestRules(), + args: args{ + rule: NewTestRule(), + }, + want: true, + }, + { + name: "ensure rule set without rule returns false", + rules: NewTestRules(), + args: args{ + rule: &Rule{ + Group: "fake", + Resource: "alsoFake", + }, + }, + want: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := tt.rules.hasResourceRule(tt.args.rule); got != tt.want { + t.Errorf("Rules.hasResourceRule() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestRules_addForManifest(t *testing.T) { + t.Parallel() + + empty := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRole", + "metadata": map[string]interface{}{ + "name": "clusterrole", + }, + }, + } + + invalidRules := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRole", + "metadata": map[string]interface{}{ + "name": "clusterrole", + }, + "rules": map[string]interface{}{ + "apiGroups": "whoops", + }, + }, + } + + invalidRule := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRole", + "metadata": map[string]interface{}{ + "name": "clusterrole", + }, + "rules": []interface{}{ + map[string]interface{}{ + "apiGroups": "whoops", + "resources": []interface{}{ + "services", + }, + "verbs": []interface{}{ + "get", + }, + }, + }, + }, + } + + resource := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Service", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "contour-svc", + "namespace": "default", + }, + "spec": map[string]interface{}{ + "selector": map[string]interface{}{ + "app": "contour", + }, + "ports": []interface{}{ + map[string]interface{}{ + "protocol": "TCP", + "port": 80, + "targetPort": 8080, + }, + }, + }, + }, + } + + resourceRoleRule := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRole", + "metadata": map[string]interface{}{ + "name": "clusterrole", + }, + "rules": []interface{}{ + map[string]interface{}{ + "apiGroups": []interface{}{ + "group", + }, + "resources": []interface{}{ + "services", + }, + "verbs": []interface{}{ + "get", + }, + }, + }, + }, + } + + nonResourceRoleRule := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRole", + "metadata": map[string]interface{}{ + "name": "clusterrole", + }, + "rules": []interface{}{ + map[string]interface{}{ + "nonResourceURLs": []interface{}{ + "/metrics", + }, + "verbs": []interface{}{ + "get", + }, + }, + }, + }, + } + + type args struct { + manifest *unstructured.Unstructured + } + + tests := []struct { + name string + rules *Rules + args args + wantErr bool + want *Rules + }{ + { + name: "resource without rules should return no error and add the regular resource", + rules: &Rules{}, + args: args{ + manifest: empty, + }, + wantErr: false, + want: &Rules{ + { + Group: "rbac.authorization.k8s.io", + Resource: "clusterroles", + Verbs: defaultResourceVerbs(), + }, + }, + }, + { + name: "resource with invalid rule should return error and add the regular resource", + rules: &Rules{}, + args: args{ + manifest: invalidRule, + }, + wantErr: true, + want: &Rules{ + { + Group: "rbac.authorization.k8s.io", + Resource: "clusterroles", + Verbs: defaultResourceVerbs(), + }, + }, + }, + { + name: "resource with invalid rules should return error and add the regular resource", + rules: &Rules{}, + args: args{ + manifest: invalidRules, + }, + wantErr: true, + want: &Rules{ + { + Group: "rbac.authorization.k8s.io", + Resource: "clusterroles", + Verbs: defaultResourceVerbs(), + }, + }, + }, + { + name: "add regular resource rule", + rules: &Rules{}, + args: args{ + manifest: resource, + }, + wantErr: false, + want: &Rules{ + { + Group: "core", + Resource: "services", + Verbs: defaultResourceVerbs(), + }, + }, + }, + { + name: "add non resource role rule", + rules: &Rules{}, + args: args{ + manifest: nonResourceRoleRule, + }, + wantErr: false, + want: &Rules{ + { + Group: "rbac.authorization.k8s.io", + Resource: "clusterroles", + Verbs: defaultResourceVerbs(), + }, + { + URLs: []string{"/metrics"}, + Verbs: []string{"get"}, + }, + }, + }, + { + name: "add resource role rule", + rules: &Rules{}, + args: args{ + manifest: resourceRoleRule, + }, + wantErr: false, + want: &Rules{ + { + Group: "rbac.authorization.k8s.io", + Resource: "clusterroles", + Verbs: defaultResourceVerbs(), + }, + { + Group: "group", + Resource: "services", + Verbs: []string{"get"}, + }, + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if err := tt.rules.addForManifest(tt.args.manifest); (err != nil) != tt.wantErr { + t.Errorf("Rules.addForManifest() error = %v, wantErr %v", err, tt.wantErr) + } + assert.Equal(t, tt.want, tt.rules) + }) + } +} diff --git a/internal/workload/v1/rbac_internal_test.go b/internal/workload/v1/rbac_internal_test.go deleted file mode 100644 index 855fdf5..0000000 --- a/internal/workload/v1/rbac_internal_test.go +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2021 VMware, Inc. -// SPDX-License-Identifier: MIT - -package v1 - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func NewRBACTest() *RBACRule { - return &RBACRule{ - Group: "core", - Resource: "exampleResource", - Verbs: []string{"get"}, - } -} - -func TestRBACRule_AddVerb(t *testing.T) { - t.Parallel() - - type args struct { - verb string - } - - tests := []struct { - name string - rbac *RBACRule - args args - }{ - { - name: "Test adding new verb", - rbac: NewRBACTest(), - args: args{ - verb: "delete", - }, - }, - } - - for _, tt := range tests { - tt := tt - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - r := tt.rbac - r.addVerb(tt.args.verb) - - assert.Equal(t, r.Verbs, []string{"get", "delete"}) - }) - } -} diff --git a/internal/workload/v1/resource.go b/internal/workload/v1/resource.go index 0964275..a5bc679 100644 --- a/internal/workload/v1/resource.go +++ b/internal/workload/v1/resource.go @@ -159,7 +159,7 @@ func expandResources(path string, resources []*Resource) ([]*Resource, error) { var expandedResources []*Resource for _, r := range resources { - files, err := Glob(filepath.Join(path, r.FileName)) + files, err := utils.Glob(filepath.Join(path, r.FileName)) if err != nil { return []*Resource{}, fmt.Errorf("failed to process glob pattern matching, %w", err) } diff --git a/internal/workload/v1/standalone.go b/internal/workload/v1/standalone.go index a8837a3..5c90686 100644 --- a/internal/workload/v1/standalone.go +++ b/internal/workload/v1/standalone.go @@ -11,6 +11,7 @@ import ( "github.com/vmware-tanzu-labs/operator-builder/internal/utils" "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/markers" + "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/rbac" ) var ErrNoComponentsOnStandalone = errors.New("cannot set component workloads on a component workload - only on collections") @@ -142,7 +143,7 @@ func (*StandaloneWorkload) IsCollection() bool { } func (s *StandaloneWorkload) SetRBAC() { - s.Spec.RBACRules.addRulesForWorkload(s) + s.Spec.RBACRules.Add(rbac.ForWorkloads(s)) } func (s *StandaloneWorkload) SetResources(workloadPath string) error { @@ -187,8 +188,8 @@ func (s *StandaloneWorkload) GetAPISpecFields() *APIFields { return s.Spec.APISpecFields } -func (s *StandaloneWorkload) GetRBACRules() *[]RBACRule { - var rules []RBACRule = *s.Spec.RBACRules +func (s *StandaloneWorkload) GetRBACRules() *[]rbac.Rule { + var rules []rbac.Rule = *s.Spec.RBACRules return &rules } diff --git a/internal/workload/v1/workload.go b/internal/workload/v1/workload.go index 8def939..d05ca55 100644 --- a/internal/workload/v1/workload.go +++ b/internal/workload/v1/workload.go @@ -17,6 +17,7 @@ import ( "github.com/vmware-tanzu-labs/operator-builder/internal/markers/inspect" "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/markers" + "github.com/vmware-tanzu-labs/operator-builder/internal/workload/v1/rbac" ) // WorkloadAPISpec sample fields which may be used in things like testing or @@ -53,7 +54,7 @@ type WorkloadSpec struct { Collection *WorkloadCollection `json:",omitempty" yaml:",omitempty" validate:"omitempty"` APISpecFields *APIFields `json:",omitempty" yaml:",omitempty" validate:"omitempty"` SourceFiles *[]SourceFile `json:",omitempty" yaml:",omitempty" validate:"omitempty"` - RBACRules *RBACRules `json:",omitempty" yaml:",omitempty" validate:"omitempty"` + RBACRules *rbac.Rules `json:",omitempty" yaml:",omitempty" validate:"omitempty"` } func (ws *WorkloadSpec) init() { @@ -69,7 +70,7 @@ func (ws *WorkloadSpec) init() { ws.appendCollectionRef() } - ws.RBACRules = &RBACRules{} + ws.RBACRules = &rbac.Rules{} ws.SourceFiles = &[]SourceFile{} } @@ -167,35 +168,40 @@ func (ws *WorkloadSpec) processManifests(markerTypes ...markers.MarkerType) erro decoder := serializer.NewCodecFactory(scheme.Scheme).UniversalDecoder() - err := runtime.DecodeInto(decoder, []byte(manifest), &manifestObject) - if err != nil { + if err := runtime.DecodeInto(decoder, []byte(manifest), &manifestObject); err != nil { return formatProcessError(manifestFile.FileName, err) } - // generate a unique name for the resource using the kind and name - resourceUniqueName := generateUniqueResourceName(manifestObject) - - // determine resource group and version - resourceVersion, resourceGroup := versionGroupFromAPIVersion(manifestObject.GetAPIVersion()) - // add the rules for this manifest - err = ws.RBACRules.addRulesForManifest(manifestObject.GetKind(), resourceGroup, manifestObject.Object) + rules, err := rbac.ForManifest(&manifestObject) if err != nil { - return err + return fmt.Errorf( + "%w; error generating rbac for kind [%s] with name [%s]", + formatProcessError(manifestFile.FileName, err), + manifestObject.GetKind(), + manifestObject.GetName(), + ) } + ws.RBACRules.Add(rules) + resource := ChildResource{ Name: manifestObject.GetName(), - UniqueName: resourceUniqueName, - Group: resourceGroup, - Version: resourceVersion, + UniqueName: generateUniqueResourceName(manifestObject), + Group: manifestObject.GetObjectKind().GroupVersionKind().Group, + Version: manifestObject.GetObjectKind().GroupVersionKind().Version, Kind: manifestObject.GetKind(), } // generate the object source code resourceDefinition, err := generate.Generate([]byte(manifest), "resourceObj") if err != nil { - return formatProcessError(manifestFile.FileName, err) + return fmt.Errorf( + "%w; error generating resource definition for kind [%s] with name [%s]", + formatProcessError(manifestFile.FileName, err), + manifestObject.GetKind(), + manifestObject.GetName(), + ) } // add the source code to the resource @@ -368,7 +374,7 @@ func (ws *WorkloadSpec) needsCollectionRef() bool { } func formatProcessError(manifestFile string, err error) error { - return fmt.Errorf("error processing file %s; %w", manifestFile, err) + return fmt.Errorf("%w; error processing file %s", err, manifestFile) } func generateUniqueResourceName(object unstructured.Unstructured) string { @@ -394,17 +400,3 @@ func generateUniqueResourceName(object unstructured.Unstructured) string { return resourceName } - -func versionGroupFromAPIVersion(apiVersion string) (version, group string) { - apiVersionElements := strings.Split(apiVersion, "/") - - if len(apiVersionElements) == 1 { - version = apiVersionElements[0] - group = coreRBACGroup - } else { - version = apiVersionElements[1] - group = rbacGroupFromGroup(apiVersionElements[0]) - } - - return version, group -} diff --git a/test/cases/edge-collection/.workloadConfig/ingress/contour-component.yaml b/test/cases/edge-collection/.workloadConfig/ingress/contour-component.yaml index e2e91e0..1ed08fd 100644 --- a/test/cases/edge-collection/.workloadConfig/ingress/contour-component.yaml +++ b/test/cases/edge-collection/.workloadConfig/ingress/contour-component.yaml @@ -13,6 +13,7 @@ spec: # name: contour # description: Manage contour component resources: + - rbac.yaml - ingress-ns.yaml - contour-config.yaml - contour-deploy.yaml diff --git a/test/cases/edge-collection/.workloadConfig/ingress/rbac.yaml b/test/cases/edge-collection/.workloadConfig/ingress/rbac.yaml new file mode 100644 index 0000000..165513f --- /dev/null +++ b/test/cases/edge-collection/.workloadConfig/ingress/rbac.yaml @@ -0,0 +1,21 @@ +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: this-is-a-dummy-clusterrole +rules: + - nonResourceURLs: + - /metrics + verbs: + - get +# NOTE: this is invalid. roles do not have non resource urls as they are not namespaced. +# --- +# apiVersion: rbac.authorization.k8s.io/v1 +# kind: Role +# metadata: +# name: this-is-a-dummy-role +# rules: +# - nonResourceURLs: +# - /metrics +# verbs: +# - get