fix: Fixes #255 removed all prepending _ from generated file names (#261

)

* fix: Fixes #255 removed all prepending _ from generated file names

Signed-off-by: Dustin Scott <sdustin@vmware.com>

* test: updated test cases to ensure passing

Signed-off-by: Dustin Scott <sdustin@vmware.com>

* chore: Fixes #264, added make target for linter install and linting

Signed-off-by: Dustin Scott <sdustin@vmware.com>

* refactor: refactor to fix linter rules for new version of golangci-lint

Signed-off-by: Dustin Scott <sdustin@vmware.com>

.github/workflows/check.yaml

@@ -95,5 +95,5 @@ jobs:
       - name: Lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: latest
+          version: v1.44.0
           args: --timeout 3m

Makefile

@@ -33,6 +33,14 @@ install: build
 #
 # traditional testing
 #
+GOLANGCI_LINT_VERSION ?= v1.44.0
+install-linter:
+	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | \
+		sh -s -- -b $$(go env GOPATH)/bin $(GOLANGCI_LINT_VERSION)
+
+lint:
+	golangci-lint run
+
 test:
 	go test -cover -coverprofile=./bin/coverage.out ./...
 

internal/workload/v1/init_config.go

@@ -13,12 +13,9 @@ import (
 )
 
 var (
-	ErrConvertArrayInterface     = errors.New("unable to convert to []interface{}")
-	ErrConvertMapStringInterface = errors.New("unable to convert to map[string]interface{}")
-	ErrConvertString             = errors.New("unable to convert to string")
-	ErrFileExists                = errors.New("force was not requested and file exists")
-	ErrWriteFile                 = errors.New("unable to write to file")
-	ErrWriteStdout               = errors.New("unable to write to stdout")
+	ErrFileExists  = errors.New("force was not requested and file exists")
+	ErrWriteFile   = errors.New("unable to write to file")
+	ErrWriteStdout = errors.New("unable to write to stdout")
 )
 
 const (
@@ -120,27 +117,27 @@ func mutateConfig(workloadConfig WorkloadInitializer) ([]byte, error) {
 }
 
 func mutateResources(yamlData map[string]interface{}) error {
-	specField, ok := yamlData["spec"].(map[string]interface{})
-	if !ok {
-		return fmt.Errorf("%w; error converting workload config spec", ErrConvertMapStringInterface)
+	specField, err := toMapStringInterface(yamlData["spec"])
+	if err != nil {
+		return fmt.Errorf("%w; error converting workload config spec %v", err, yamlData["spec"])
 	}
 
-	resourceObjs, ok := specField["resources"].([]interface{})
-	if !ok {
-		return fmt.Errorf("%w; error converting spec.resources", ErrConvertArrayInterface)
+	resourceObjs, err := toArrayInterface(specField["resources"])
+	if err != nil {
+		return fmt.Errorf("%w; error converting spec.resources %v", err, specField["resources"])
 	}
 
 	resources := make([]string, len(resourceObjs))
 
 	for i, resource := range resourceObjs {
-		resourceMap, ok := resource.(map[string]interface{})
-		if !ok {
-			return fmt.Errorf("%w; error converting spec.resources item", ErrConvertMapStringInterface)
+		resourceMap, err := toMapStringInterface(resource)
+		if err != nil {
+			return fmt.Errorf("%w; error converting spec.resources item %v", err, resource)
 		}
 
-		filename, ok := resourceMap["filename"].(string)
-		if !ok {
-			return fmt.Errorf("%w; error converting spec.resources.filename", ErrConvertString)
+		filename, err := toString(resourceMap["filename"])
+		if err != nil {
+			return fmt.Errorf("%w; error converting spec.resources.filename %v", err, resource)
 		}
 
 		resources[i] = filename

internal/workload/v1/rbac.go

@@ -23,6 +23,16 @@ type RBACRule struct {
 	VerbString 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    []string
+	Resources []string
+	Verbs     []string
+}
+
 type RBACRules []RBACRule
 
 func (r *RBACRule) AddVerb(verb string) {
@@ -78,10 +88,6 @@ func (rs *RBACRules) groupResourceRecorded(newRBACRule *RBACRule) bool {
 }
 
 func (rs *RBACRules) AddOrUpdateRules(newRule *RBACRule) {
-	if rs == nil {
-		rs = &RBACRules{}
-	}
-
 	if !rs.groupResourceRecorded(newRule) {
 		newRule.VerbString = rbacVerbsToString(newRule.Verbs)
 		*rs = append(*rs, *newRule)
@@ -97,6 +103,33 @@ func (rs *RBACRules) AddOrUpdateRules(newRule *RBACRule) {
 	}
 }
 
+func (rs *RBACRules) AddOrUpdateRoleRules(newRule *RBACRoleRule) {
+	// assign a new rule for each group and kind match
+	if len(newRule.Groups) == 0 {
+		return
+	}
+
+	for _, rbacGroup := range newRule.Groups {
+		if len(newRule.Resources) == 0 {
+			return
+		}
+
+		for _, rbacKind := range newRule.Resources {
+			if len(newRule.Verbs) == 0 {
+				return
+			}
+
+			rs.AddOrUpdateRules(
+				&RBACRule{
+					Group:    rbacGroupFromGroup(rbacGroup),
+					Resource: getResourceForRBAC(rbacKind),
+					Verbs:    newRule.Verbs,
+				},
+			)
+		}
+	}
+}
+
 func getResourceForRBAC(kind string) string {
 	rbacResource := strings.Split(kind, "/")
 
@@ -139,7 +172,7 @@ func valueFromInterface(in interface{}, key string) (out interface{}) {
 	return out
 }
 
-func (rs *RBACRules) addRulesForManifest(kind, group string, rawContent interface{}) {
+func (rs *RBACRules) addRulesForManifest(kind, group string, rawContent interface{}) error {
 	rs.AddOrUpdateRules(
 		&RBACRule{
 			Group:    group,
@@ -151,47 +184,50 @@ func (rs *RBACRules) addRulesForManifest(kind, group string, rawContent interfac
 	// 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") {
-		resourceRules := valueFromInterface(rawContent, "rules")
-		if resourceRules == nil {
-			return
+		rules := valueFromInterface(rawContent, "rules")
+		if rules == nil {
+			return nil
 		}
 
-		for _, resourceRule := range resourceRules.([]interface{}) {
-			rbacGroups := valueFromInterface(resourceRule, "apiGroups")
-			rbacKinds := valueFromInterface(resourceRule, "resources")
-			rbacVerbs := valueFromInterface(resourceRule, "verbs")
+		rbacRoleRules, err := toArrayInterface(rules)
+		if err != nil {
+			return fmt.Errorf("%w; error converting resource rules %v", err, rules)
+		}
 
-			// assign a new rule for each group and kind match
-			if rbacGroups == nil {
-				continue
+		for _, rbacRoleRule := range rbacRoleRules {
+			rule := &RBACRoleRule{}
+			if err := rule.processRawRule(rbacRoleRule); err != nil {
+				return fmt.Errorf("%w; error processing rbac role rule %v", err, rules)
 			}
 
-			for _, rbacGroup := range rbacGroups.([]interface{}) {
-				if rbacKinds == nil {
-					continue
-				}
-
-				for _, rbacKind := range rbacKinds.([]interface{}) {
-					if rbacVerbs == nil {
-						continue
-					}
-					// gather verbs and convert to strings
-					var verbs []string
-					for _, verb := range rbacVerbs.([]interface{}) {
-						verbs = append(verbs, verb.(string))
-					}
-
-					rs.AddOrUpdateRules(
-						&RBACRule{
-							Group:    rbacGroupFromGroup(rbacGroup.(string)),
-							Resource: getResourceForRBAC(rbacKind.(string)),
-							Verbs:    verbs,
-						},
-					)
-				}
-			}
+			rs.AddOrUpdateRoleRules(rule)
 		}
 	}
+
+	return nil
+}
+
+func (roleRule *RBACRoleRule) processRawRule(rule interface{}) error {
+	rbacGroups, err := toArrayString(valueFromInterface(rule, "apiGroups"))
+	if err != nil {
+		return fmt.Errorf("%w; error converting rbac groups for rule %v", err, rule)
+	}
+
+	rbacKinds, err := toArrayString(valueFromInterface(rule, "resources"))
+	if err != nil {
+		return fmt.Errorf("%w; error converting rbac kinds for rule %v", err, rule)
+	}
+
+	rbacVerbs, err := toArrayString(valueFromInterface(rule, "verbs"))
+	if err != nil {
+		return fmt.Errorf("%w; error converting rbac verbs for rule %v", err, rule)
+	}
+
+	roleRule.Groups = rbacGroups
+	roleRule.Resources = rbacKinds
+	roleRule.Verbs = rbacVerbs
+
+	return nil
 }
 
 func defaultResourceVerbs() []string {

internal/workload/v1/resource.go

@@ -136,6 +136,15 @@ func determineSourceFileName(manifestFile string) SourceFile {
 	sourceFile.Filename += ".go"                                                                         // add correct file ext
 	sourceFile.Filename = utils.ToFileName(sourceFile.Filename)                                          // kebab-case to snake_case
 
+	// strip any prefix that begins with _ or even multiple _s because go does not recognize these files
+	for _, char := range sourceFile.Filename {
+		if string(char) == "_" {
+			sourceFile.Filename = strings.TrimPrefix(sourceFile.Filename, "_")
+		} else {
+			break
+		}
+	}
+
 	return sourceFile
 }
 

internal/workload/v1/utils.go

@@ -4,6 +4,7 @@
 package v1
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -12,6 +13,13 @@ 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 {
@@ -89,3 +97,39 @@ 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) {
+	out, ok := in.([]string)
+	if !ok {
+		return nil, ErrConvertArrayString
+	}
+
+	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
+}

internal/workload/v1/workload.go

@@ -176,8 +176,11 @@ func (ws *WorkloadSpec) processManifests(markerTypes ...MarkerType) error {
 			// determine resource group and version
 			resourceVersion, resourceGroup := versionGroupFromAPIVersion(manifestObject.GetAPIVersion())
 
-			// determine group and resource for RBAC rule generation
-			ws.RBACRules.addRulesForManifest(manifestObject.GetKind(), resourceGroup, manifestObject.Object)
+			// add the rules for this manifest
+			err = ws.RBACRules.addRulesForManifest(manifestObject.GetKind(), resourceGroup, manifestObject.Object)
+			if err != nil {
+				return err
+			}
 
 			ws.OwnershipRules.addOrUpdateOwnership(
 				manifestObject.GetAPIVersion(),

test/cases/edge-collection/.workloadConfig/resource-up-one-level.yaml

@@ -0,0 +1,5 @@
+---
+apiVersion: v1
+kind: Namespace
+metadata:
+  name: this-is-a-test-resource-up-one-level

test/cases/edge-collection/.workloadConfig/tenancy/ns-operator-component.yaml

@@ -12,5 +12,6 @@ spec:
   resources:
   - ns-operator-crd.yaml
   - ns-operator-deploy.yaml
+  - ../resource-up-one-level.yaml
   dependencies:
   - tenancy-common-component