diff --git a/internal/plugins/workload/v1/scaffolds/templates/controller/controller.go b/internal/plugins/workload/v1/scaffolds/templates/controller/controller.go index 17bc0707..14e71b0e 100644 --- a/internal/plugins/workload/v1/scaffolds/templates/controller/controller.go +++ b/internal/plugins/workload/v1/scaffolds/templates/controller/controller.go @@ -24,6 +24,12 @@ type Controller struct { // input fields Builder workloadv1.WorkloadAPIBuilder + + // template fields + BaseImports []string + OtherImports []string + InternalImports []string + CollectionImports []string } func (f *Controller) SetTemplateDefaults() error { @@ -36,40 +42,146 @@ func (f *Controller) SetTemplateDefaults() error { f.TemplateBody = controllerTemplate f.IfExistsAction = machinery.OverwriteFile + f.setBaseImports() + f.setOtherImports() + f.setInternalImports() + + if f.Builder.IsCollection() { + f.setCollectionImports() + } + return nil } +func (f *Controller) setBaseImports() { + f.BaseImports = []string{`"context"`, `"fmt"`} + + if f.Builder.IsComponent() { + f.BaseImports = append(f.BaseImports, `"errors"`, `"reflect"`) + } +} + +func (f *Controller) setOtherImports() { + f.OtherImports = []string{ + `"github.com/go-logr/logr"`, + `apierrs "k8s.io/apimachinery/pkg/api/errors"`, + `"k8s.io/client-go/tools/record"`, + `ctrl "sigs.k8s.io/controller-runtime"`, + `"sigs.k8s.io/controller-runtime/pkg/client"`, + `"sigs.k8s.io/controller-runtime/pkg/controller"`, + `"github.com/nukleros/operator-builder-tools/pkg/controller/phases"`, + `"github.com/nukleros/operator-builder-tools/pkg/controller/predicates"`, + `"github.com/nukleros/operator-builder-tools/pkg/controller/workload"`, + } + + if f.Builder.IsComponent() { + f.OtherImports = append(f.OtherImports, + `"github.com/nukleros/operator-builder-tools/pkg/resources"`, + `"sigs.k8s.io/controller-runtime/pkg/event"`, + `"sigs.k8s.io/controller-runtime/pkg/handler"`, + `"sigs.k8s.io/controller-runtime/pkg/predicate"`, + `"sigs.k8s.io/controller-runtime/pkg/reconcile"`, + `"sigs.k8s.io/controller-runtime/pkg/source"`, + `"k8s.io/apimachinery/pkg/types"`, + ) + } +} + +func (f *Controller) setInternalImports() { + f.InternalImports = []string{ + fmt.Sprintf(`"%s/internal/dependencies"`, f.Repo), + fmt.Sprintf(`"%s/internal/mutate"`, f.Repo), + fmt.Sprintf(`%s %q`, f.Resource.ImportAlias(), f.Resource.Path), + } + + if f.Builder.IsComponent() { + f.InternalImports = append(f.InternalImports, f.getAPITypesPath(f.Builder.GetCollection())) + } + + if f.Builder.HasChildResources() { + f.InternalImports = append(f.InternalImports, + fmt.Sprintf(`"%s/%s"`, + f.Resource.Path, + f.Builder.GetPackageName(), + ), + ) + } +} + +func (f *Controller) setCollectionImports() { + for _, component := range f.Builder.GetComponents() { + if !f.importIsDefined(f.getAPITypesPath(component)) { + f.CollectionImports = append(f.CollectionImports, f.getAPITypesPath(component)) + } + } + + f.deduplicateCollectionImports() +} + +func (f *Controller) importIsDefined(importCheck string) bool { + existingImports := []string{} + existingImports = append(existingImports, f.BaseImports...) + existingImports = append(existingImports, f.OtherImports...) + existingImports = append(existingImports, f.InternalImports...) + + for _, existing := range existingImports { + if importCheck == existing { + return true + } + } + + return false +} + +func (f *Controller) deduplicateCollectionImports() { + keys := make(map[string]bool) + + collectionImports := []string{} + + for _, existing := range f.CollectionImports { + if _, value := keys[existing]; !value { + keys[existing] = true + + collectionImports = append(collectionImports, existing) + } + } + + f.CollectionImports = collectionImports +} + +func (f *Controller) getAPITypesPath(builder workloadv1.WorkloadAPIBuilder) string { + return fmt.Sprintf(`%s%s "%s/apis/%s/%s"`, + builder.GetAPIGroup(), + builder.GetAPIVersion(), + f.Repo, + builder.GetAPIGroup(), + builder.GetAPIVersion(), + ) +} + //nolint: lll const controllerTemplate = `{{ .Boilerplate }} package {{ .Resource.Group }} import ( - "context" - {{- if .Builder.IsComponent }} - "errors" - {{- end }} - "fmt" + {{ range .BaseImports -}} + {{ . }} + {{ end }} + + {{ range .OtherImports -}} + {{ . }} + {{ end }} - "github.com/go-logr/logr" - "github.com/nukleros/operator-builder-tools/pkg/controller/phases" - "github.com/nukleros/operator-builder-tools/pkg/controller/predicates" - "github.com/nukleros/operator-builder-tools/pkg/controller/workload" - apierrs "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/client-go/tools/record" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller" - - {{ .Resource.ImportAlias }} "{{ .Resource.Path }}" - {{ if .Builder.IsComponent -}} - {{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }} "{{ .Repo }}/apis/{{ .Builder.GetCollection.Spec.API.Group }}/{{ .Builder.GetCollection.Spec.API.Version }}" + {{ range .InternalImports -}} + {{ . }} + {{ end }} + + {{ if .Builder.IsCollection -}} + {{ range .CollectionImports -}} + {{ . }} + {{ end }} {{ end }} - {{- if .Builder.HasChildResources -}} - "{{ .Resource.Path }}/{{ .Builder.GetPackageName }}" - {{ end -}} - "{{ .Repo }}/internal/dependencies" - "{{ .Repo }}/internal/mutate" ) // {{ .Resource.Kind }}Reconciler reconciles a {{ .Resource.Kind }} object. @@ -96,10 +208,8 @@ func New{{ .Resource.Kind }}Reconciler(mgr ctrl.Manager) *{{ .Resource.Kind }}Re } } -// +kubebuilder:rbac:groups={{ .Resource.Group }}.{{ .Resource.Domain }},resources={{ .Resource.Plural }},verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups={{ .Resource.Group }}.{{ .Resource.Domain }},resources={{ .Resource.Plural }}/status,verbs=get;update;patch {{ range .Builder.GetRBACRules -}} -// +kubebuilder:rbac:groups={{ .Group }},resources={{ .Resource }},verbs={{ .VerbString }} +{{ .ToMarker }} {{ end }} // Until Webhooks are implemented we need to list and watch namespaces to ensure @@ -177,65 +287,119 @@ func (r *{{ .Resource.Kind }}Reconciler) NewRequest(ctx context.Context, request {{- if .Builder.IsComponent }} // SetCollection sets the collection for a particular workload request. func (r *{{ .Resource.Kind }}Reconciler) SetCollection(component *{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}, req *workload.Request) error { - // get and store the collection + collection, err := r.GetCollection(component, req) + if err != nil || collection == nil { + return fmt.Errorf("unable to set collection, %w", err) + } + + req.Collection = collection + + // set the owner reference so that we can reconcile this workload on changes to the collection + if err := ctrl.SetControllerReference(collection, req.Workload, r.Scheme()); err != nil { + req.Log.Error( + err, "unable to set collection owner reference on component workload", + "Name", req.Workload.GetName(), + "Namespace", req.Workload.GetNamespace(), + "collection.Name", collection.GetName(), + "collection.Namespace", collection.GetNamespace(), + ) + + return fmt.Errorf("unable to set owner reference on %s, %w", req.Workload.GetName(), err) + } + + return r.EnqueueRequestOnCollectionChange(req) +} + +// GetCollection gets a collection for a component given a list. +func (r *{{ .Resource.Kind }}Reconciler) GetCollection( + component *{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}, + req *workload.Request, +) (*{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }}, error) { var collectionList {{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }}List if err := r.List(req.Context, &collectionList); err != nil { - return fmt.Errorf("unable to list collection {{ .Builder.GetCollection.Spec.API.Kind }}, %w", err) + return nil, fmt.Errorf("unable to list collection {{ .Builder.GetCollection.Spec.API.Kind }}, %w", err) } // determine if we have requested a specific collection - var collectionRef {{ .Resource.ImportAlias }}.{{ .Resource.Kind }}CollectionSpec + name, namespace := component.Spec.Collection.Name, component.Spec.Collection.Namespace - collectionRequested := component.Spec.Collection != collectionRef && component.Spec.Collection.Name != "" + var collectionRef {{ .Resource.ImportAlias }}.{{ .Resource.Kind }}CollectionSpec - switch len(collectionList.Items) { - case 0: - if component.GetDeletionTimestamp().IsZero() { - req.Log.Info("no collections available; initiating controller requeue") + hasSpecificCollection := component.Spec.Collection != collectionRef && component.Spec.Collection.Name != "" - return workload.ErrCollectionNotFound - } - case 1: - if collectionRequested { - if collection := r.GetCollection(component, collectionList); collection != nil { - req.Collection = collection - } else { - return fmt.Errorf("no valid {{ .Builder.GetCollection.Spec.API.Kind }} collections found in namespace %s with name %s", - component.Spec.Collection.Namespace, component.Spec.Collection.Name) - } - } - - req.Collection = &collectionList.Items[0] - default: - if collectionRequested { - if collection := r.GetCollection(component, collectionList); collection != nil { - req.Collection = collection - } else { - return fmt.Errorf("no valid {{ .Builder.GetCollection.Spec.API.Kind }} collections found in namespace %s with name %s", - component.Spec.Collection.Namespace, component.Spec.Collection.Name) - } + // if a specific collection has not been requested, we ensure only one exists + if !hasSpecificCollection { + if len(collectionList.Items) != 1 { + return nil, fmt.Errorf("expected only 1 {{ .Builder.GetCollection.Spec.API.Kind }} collection, found %v", len(collectionList.Items)) } - return fmt.Errorf("multiple valid collections found; expected 1; cannot proceed") + return &collectionList.Items[0], nil } - return nil + // find the collection that was requested and return it + for _, collection := range collectionList.Items { + if collection.Name == name && collection.Namespace == namespace { + return &collection, nil + } + } + + return nil, workload.ErrCollectionNotFound } -// GetCollection gets a collection for a component given a list. -func (r *{{ .Resource.Kind }}Reconciler) GetCollection( - component *{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}, - collectionList {{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }}List, -) *{{ .Builder.GetCollection.Spec.API.Group }}{{ .Builder.GetCollection.Spec.API.Version }}.{{ .Builder.GetCollection.Spec.API.Kind }} { - name, namespace := component.Spec.Collection.Name, component.Spec.Collection.Namespace +// EnqueueRequestOnCollectionChange enqueues a reconcile request when an associated collection object changes. +func (r *{{ .Resource.Kind }}Reconciler) EnqueueRequestOnCollectionChange(req *workload.Request) error { + if len(r.Watches) > 0 { + for _, watched := range r.Watches { + if reflect.DeepEqual( + req.Collection.GetObjectKind().GroupVersionKind(), + watched.GetObjectKind().GroupVersionKind(), + ) { + return nil + } + } + } - for _, collection := range collectionList.Items { - if collection.Name == name && collection.Namespace == namespace { - return &collection + // create a function which maps this specific reconcile request + mapFn := func(collection client.Object) []reconcile.Request { + return []reconcile.Request{ + { + NamespacedName: types.NamespacedName{ + Name: req.Workload.GetName(), + Namespace: req.Workload.GetNamespace(), + }, + }, } } + // watch the collection and use our map function to enqueue the request + if err := r.Controller.Watch( + &source.Kind{Type: req.Collection}, + handler.EnqueueRequestsFromMapFunc(mapFn), + predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + if !resources.EqualNamespaceName(e.ObjectNew, req.Collection) { + return false + } + + return e.ObjectNew != e.ObjectOld + }, + CreateFunc: func(e event.CreateEvent) bool { + return false + }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, + }, + ); err != nil { + return err + } + + r.Watches = append(r.Watches, req.Collection) + return nil } {{- end }} @@ -330,6 +494,11 @@ func (r *{{ .Resource.Kind }}Reconciler) SetupWithManager(mgr ctrl.Manager) erro baseController, err := ctrl.NewControllerManagedBy(mgr). WithEventFilter(predicates.WorkloadPredicates()). For(&{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}{}). + {{ if .Builder.IsCollection -}} + {{ range .Builder.GetComponents -}} + Owns(&{{ .Spec.API.Group }}{{ .Spec.API.Version }}.{{ .Spec.API.Kind }}{}). + {{ end -}} + {{ end -}} Build(r) if err != nil { return fmt.Errorf("unable to setup controller, %w", err) diff --git a/internal/plugins/workload/v1/scaffolds/templates/test/e2e/e2e.go b/internal/plugins/workload/v1/scaffolds/templates/test/e2e/e2e.go index 026363ea..22d53963 100644 --- a/internal/plugins/workload/v1/scaffolds/templates/test/e2e/e2e.go +++ b/internal/plugins/workload/v1/scaffolds/templates/test/e2e/e2e.go @@ -160,7 +160,9 @@ func TestMain(t *testing.T) { collectionSuite.teardown() // check all controller logs for errors - require.NoErrorf(t, testControllerLogsNoErrors(e2eTestSuite, ""), "found errors in controller logs") + if os.Getenv("DEPLOY_IN_CLUSTER") == "true" { + require.NoErrorf(t, testControllerLogsNoErrors(e2eTestSuite, ""), "found errors in controller logs") + } // perform final teardown require.NoErrorf(t, finalTeardown(), "error tearing down test suite") diff --git a/internal/workload/v1/api.go b/internal/workload/v1/api.go index 557a7db4..ab364909 100644 --- a/internal/workload/v1/api.go +++ b/internal/workload/v1/api.go @@ -176,14 +176,6 @@ func (api *APIFields) generateAPISpecField(b io.StringWriter, kind string) { func (api *APIFields) generateAPIStruct(b io.StringWriter, kind string) { if api.Type == FieldStruct { - for _, m := range api.Markers { - mustWrite(b.WriteString(fmt.Sprintf("//%s\n", m))) - } - - for _, c := range api.Comments { - mustWrite(b.WriteString(fmt.Sprintf("//%s\n", c))) - } - mustWrite(b.WriteString(fmt.Sprintf("type %s %s{\n", kind+api.StructName, api.Type.String()))) for _, child := range api.Children { diff --git a/internal/workload/v1/collection.go b/internal/workload/v1/collection.go index fd77b394..bd5dcd0d 100644 --- a/internal/workload/v1/collection.go +++ b/internal/workload/v1/collection.go @@ -118,6 +118,10 @@ func (c *WorkloadCollection) GetPackageName() string { return c.PackageName } +func (c *WorkloadCollection) GetAPISpec() WorkloadAPISpec { + return c.Spec.API +} + func (c *WorkloadCollection) GetAPIGroup() string { return c.Spec.API.Group } @@ -146,6 +150,10 @@ func (c *WorkloadCollection) IsCollection() bool { return true } +func (c *WorkloadCollection) SetRBAC() { + c.Spec.RBACRules.addRulesForWorkload(c) +} + func (c *WorkloadCollection) SetResources(workloadPath string) error { err := c.Spec.processManifests(FieldMarkerType, CollectionMarkerType) if err != nil { @@ -205,21 +213,10 @@ func (c *WorkloadCollection) GetRBACRules() *[]RBACRule { return &rules } -func (*WorkloadCollection) GetOwnershipRules() *[]OwnershipRule { - return &[]OwnershipRule{} -} - func (c *WorkloadCollection) GetComponentResource(domain, repo string, clusterScoped bool) *resource.Resource { - var namespaced bool - if clusterScoped { - namespaced = false - } else { - namespaced = true - } - api := resource.API{ CRDVersion: "v1", - Namespaced: namespaced, + Namespaced: !clusterScoped, } return &resource.Resource{ diff --git a/internal/workload/v1/component.go b/internal/workload/v1/component.go index 67a925e0..7c6e3fb0 100644 --- a/internal/workload/v1/component.go +++ b/internal/workload/v1/component.go @@ -100,6 +100,10 @@ func (c *ComponentWorkload) GetPackageName() string { return c.PackageName } +func (c *ComponentWorkload) GetAPISpec() WorkloadAPISpec { + return c.Spec.API +} + func (c *ComponentWorkload) GetAPIGroup() string { return c.Spec.API.Group } @@ -128,6 +132,10 @@ func (*ComponentWorkload) IsCollection() bool { return false } +func (c *ComponentWorkload) SetRBAC() { + c.Spec.RBACRules.addRulesForWorkload(c) +} + func (c *ComponentWorkload) SetResources(workloadPath string) error { err := c.Spec.processManifests(FieldMarkerType) if err != nil { @@ -175,23 +183,10 @@ func (c *ComponentWorkload) GetRBACRules() *[]RBACRule { return &rules } -func (c *ComponentWorkload) GetOwnershipRules() *[]OwnershipRule { - var rules []OwnershipRule = *c.Spec.OwnershipRules - - return &rules -} - func (c *ComponentWorkload) GetComponentResource(domain, repo string, clusterScoped bool) *resource.Resource { - var namespaced bool - if clusterScoped { - namespaced = false - } else { - namespaced = true - } - api := resource.API{ CRDVersion: "v1", - Namespaced: namespaced, + Namespaced: !clusterScoped, } return &resource.Resource{ diff --git a/internal/workload/v1/config.go b/internal/workload/v1/config.go index e294ffb7..04fe4055 100644 --- a/internal/workload/v1/config.go +++ b/internal/workload/v1/config.go @@ -112,6 +112,7 @@ func ProcessAPIConfig(workloadConfig string) (WorkloadAPIBuilder, error) { } workload.SetNames() + workload.SetRBAC() markers := &markerCollection{ fieldMarkers: []*FieldMarker{}, @@ -121,12 +122,14 @@ func ProcessAPIConfig(workloadConfig string) (WorkloadAPIBuilder, error) { for _, component := range components { // assign values related to the collection component.Spec.Collection = collection + component.Spec.API.Domain = collection.Spec.API.Domain if err := component.SetResources(component.Spec.ConfigPath); err != nil { return nil, err } component.SetNames() + component.SetRBAC() markers.fieldMarkers = append(markers.fieldMarkers, component.Spec.FieldMarkers...) markers.collectionFieldMarkers = append(markers.collectionFieldMarkers, component.Spec.CollectionFieldMarkers...) diff --git a/internal/workload/v1/interfaces.go b/internal/workload/v1/interfaces.go index 4d669f53..7a32c7b2 100644 --- a/internal/workload/v1/interfaces.go +++ b/internal/workload/v1/interfaces.go @@ -52,13 +52,13 @@ type WorkloadAPIBuilder interface { GetSourceFiles() *[]SourceFile GetAPISpecFields() *APIFields GetRBACRules() *[]RBACRule - GetOwnershipRules() *[]OwnershipRule GetComponentResource(domain, repo string, clusterScoped bool) *resource.Resource GetFuncNames() (createFuncNames, initFuncNames []string) GetRootCommand() *CliCommand GetSubCommand() *CliCommand SetNames() + SetRBAC() SetResources(workloadPath string) error SetComponents(components []*ComponentWorkload) error diff --git a/internal/workload/v1/ownership.go b/internal/workload/v1/ownership.go deleted file mode 100644 index e87b733f..00000000 --- a/internal/workload/v1/ownership.go +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright 2021 VMware, Inc. -// SPDX-License-Identifier: MIT - -package v1 - -import "strings" - -// OwnershipRule contains the info needed to create the controller ownership -// functionality when setting up the controller with the manager. This allows -// the controller to reconcile the state of a deleted resource that it manages. -type OwnershipRule struct { - Version string - Kind string - CoreAPI bool -} - -type OwnershipRules []OwnershipRule - -func (or *OwnershipRules) addOrUpdateOwnership(version, kind, group string) { - if or == nil { - or = &OwnershipRules{} - } - // determine group and kind for ownership rule generation - newOwnershipRule := OwnershipRule{ - Version: version, - Kind: kind, - CoreAPI: isCoreAPI(group), - } - - if !or.versionKindRecorded(&newOwnershipRule) { - *or = append(*or, newOwnershipRule) - } -} - -func (or *OwnershipRules) versionKindRecorded(newOwnershipRule *OwnershipRule) bool { - for _, r := range *or { - if r.Version == newOwnershipRule.Version && r.Kind == newOwnershipRule.Kind { - return true - } - } - - return false -} - -func coreAPIs() []string { - return []string{ - "apps", "batch", "autoscaling", "extensions", "policy", - } -} - -func isCoreAPI(group string) bool { - // return if the group is missing or labeled as core - if group == "" || group == "core" { - return true - } - - // return if the group contains the kubernetes api group strings - if strings.Contains(group, "k8s.io") || strings.Contains(group, "kubernetes.io") { - return true - } - - // loop through known groups and return true if found - for _, coreGroup := range coreAPIs() { - if group == coreGroup { - return true - } - } - - return false -} diff --git a/internal/workload/v1/rbac.go b/internal/workload/v1/rbac.go index 6edaee2a..80367511 100644 --- a/internal/workload/v1/rbac.go +++ b/internal/workload/v1/rbac.go @@ -17,10 +17,10 @@ const ( // RBACRule contains the info needed to create the kubebuilder:rbac markers in // the controller. type RBACRule struct { - Group string - Resource string - Verbs []string - VerbString string + Group string + Resource string + Verbs []string + URLs []string } // RBACRoleRule contains the info needed to create the kubebuilder:rbac markers @@ -28,14 +28,17 @@ type RBACRule struct { // 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 + Groups RBACRoleRuleField + Resources RBACRoleRuleField + Verbs RBACRoleRuleField + URLs RBACRoleRuleField } +type RBACRoleRuleField []string + type RBACRules []RBACRule -func (r *RBACRule) AddVerb(verb string) { +func (r *RBACRule) addVerb(verb string) { var found bool for _, existingVerb := range r.Verbs { @@ -48,7 +51,6 @@ func (r *RBACRule) AddVerb(verb string) { if !found { r.Verbs = append(r.Verbs, verb) - r.VerbString = rbacVerbsToString(r.Verbs) } } @@ -60,7 +62,7 @@ func rbacGroupFromGroup(group string) string { return group } -func rbacVerbsToString(verbs []string) string { +func rbacFieldsToString(verbs []string) string { return strings.Join(verbs, ";") } @@ -87,43 +89,116 @@ func (rs *RBACRules) groupResourceRecorded(newRBACRule *RBACRule) bool { return false } -func (rs *RBACRules) AddOrUpdateRules(newRule *RBACRule) { - if !rs.groupResourceRecorded(newRule) { - newRule.VerbString = rbacVerbsToString(newRule.Verbs) +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 { - rules := *rs for i := range rules { if rules[i].groupResourceEqual(newRule) { for _, verb := range newRule.Verbs { - rules[i].AddVerb(verb) + 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) { - // assign a new rule for each group and kind match - if len(newRule.Groups) == 0 { + // we must have verbs to create our rbac + if len(newRule.Verbs) == 0 { return } - for _, rbacGroup := range newRule.Groups { - if len(newRule.Resources) == 0 { + // 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 { - if len(newRule.Verbs) == 0 { - return - } - rs.AddOrUpdateRules( &RBACRule{ Group: rbacGroupFromGroup(rbacGroup), Resource: getResourceForRBAC(rbacKind), Verbs: newRule.Verbs, + URLs: newRule.URLs, }, ) } @@ -167,11 +242,47 @@ func valueFromInterface(in interface{}, key string) (out 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{ @@ -196,8 +307,8 @@ func (rs *RBACRules) addRulesForManifest(kind, group string, rawContent interfac 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) + if err := rule.processRawRoleRule(rbacRoleRule); err != nil { + return fmt.Errorf("%w; error processing rbac role rule %v", err, rbacRoleRule) } rs.AddOrUpdateRoleRules(rule) @@ -207,29 +318,45 @@ func (rs *RBACRules) addRulesForManifest(kind, group string, rawContent interfac 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) +func (roleRule *RBACRoleRule) processRawRoleRule(rule interface{}) error { + fields := map[*RBACRoleRuleField]string{ + &roleRule.Groups: "apiGroups", + &roleRule.Resources: "resources", + &roleRule.Verbs: "verbs", + &roleRule.URLs: "nonResourceURLs", } - rbacKinds, err := toArrayString(valueFromInterface(rule, "resources")) - if err != nil { - return fmt.Errorf("%w; error converting rbac kinds for rule %v", err, rule) + for objectField, fieldKey := range fields { + if err := objectField.setRbacRoleRuleField(rule, fieldKey); err != nil { + return fmt.Errorf("%w; error processing raw fule %v", err, rule) + } } - rbacVerbs, err := toArrayString(valueFromInterface(rule, "verbs")) + 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 verbs for rule %v", err, rule) + return fmt.Errorf("%w; error converting rbac field key %s for rule %v", err, fieldKey, rule) } - roleRule.Groups = rbacGroups - roleRule.Resources = rbacKinds - roleRule.Verbs = rbacVerbs + *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_internal_test.go b/internal/workload/v1/rbac_internal_test.go index 30228d15..855fdf56 100644 --- a/internal/workload/v1/rbac_internal_test.go +++ b/internal/workload/v1/rbac_internal_test.go @@ -11,10 +11,9 @@ import ( func NewRBACTest() *RBACRule { return &RBACRule{ - Group: "core", - Resource: "exampleResource", - Verbs: []string{"get"}, - VerbString: "get", + Group: "core", + Resource: "exampleResource", + Verbs: []string{"get"}, } } @@ -44,9 +43,9 @@ func TestRBACRule_AddVerb(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() r := tt.rbac - r.AddVerb(tt.args.verb) + r.addVerb(tt.args.verb) - assert.Equal(t, r.VerbString, "get;delete") + assert.Equal(t, r.Verbs, []string{"get", "delete"}) }) } } diff --git a/internal/workload/v1/standalone.go b/internal/workload/v1/standalone.go index 8c579317..9db943d3 100644 --- a/internal/workload/v1/standalone.go +++ b/internal/workload/v1/standalone.go @@ -108,6 +108,10 @@ func (s *StandaloneWorkload) GetPackageName() string { return s.PackageName } +func (s *StandaloneWorkload) GetAPISpec() WorkloadAPISpec { + return s.Spec.API +} + func (s *StandaloneWorkload) GetAPIGroup() string { return s.Spec.API.Group } @@ -136,6 +140,10 @@ func (*StandaloneWorkload) IsCollection() bool { return false } +func (s *StandaloneWorkload) SetRBAC() { + s.Spec.RBACRules.addRulesForWorkload(s) +} + func (s *StandaloneWorkload) SetResources(workloadPath string) error { err := s.Spec.processManifests(FieldMarkerType) if err != nil { @@ -184,12 +192,6 @@ func (s *StandaloneWorkload) GetRBACRules() *[]RBACRule { return &rules } -func (s *StandaloneWorkload) GetOwnershipRules() *[]OwnershipRule { - var rules []OwnershipRule = *s.Spec.OwnershipRules - - return &rules -} - func (*StandaloneWorkload) GetComponentResource(domain, repo string, clusterScoped bool) *resource.Resource { return &resource.Resource{} } diff --git a/internal/workload/v1/workload.go b/internal/workload/v1/workload.go index 826cc528..bdb18f49 100644 --- a/internal/workload/v1/workload.go +++ b/internal/workload/v1/workload.go @@ -53,7 +53,6 @@ type WorkloadSpec struct { APISpecFields *APIFields `json:",omitempty" yaml:",omitempty" validate:"omitempty"` SourceFiles *[]SourceFile `json:",omitempty" yaml:",omitempty" validate:"omitempty"` RBACRules *RBACRules `json:",omitempty" yaml:",omitempty" validate:"omitempty"` - OwnershipRules *OwnershipRules `json:",omitempty" yaml:",omitempty" validate:"omitempty"` } func (ws *WorkloadSpec) init() { @@ -69,7 +68,6 @@ func (ws *WorkloadSpec) init() { ws.appendCollectionRef() } - ws.OwnershipRules = &OwnershipRules{} ws.RBACRules = &RBACRules{} ws.SourceFiles = &[]SourceFile{} } @@ -97,7 +95,7 @@ func (ws *WorkloadSpec) appendCollectionRef() { collectionField := &APIFields{ Name: "Collection", Type: FieldStruct, - Tags: fmt.Sprintf("`json:%q`", "collection"), + Tags: fmt.Sprintf("`json:%q`", "collection,omitempty"), Sample: "#collection:", StructName: "CollectionSpec", Markers: []string{ @@ -108,22 +106,26 @@ func (ws *WorkloadSpec) appendCollectionRef() { "workload collection in the cluster, which will result in an error", "if not exactly one collection is found.", }, + Comments: nil, Children: []*APIFields{ { - Name: "Name", - Type: FieldString, - Tags: fmt.Sprintf("`json:%q`", "name"), - Sample: fmt.Sprintf("#name: %q", strings.ToLower(ws.Collection.GetAPIKind())+"-sample"), + Name: "Name", + Type: FieldString, + Tags: fmt.Sprintf("`json:%q`", "name,omitempty"), + Sample: fmt.Sprintf("#name: %q", strings.ToLower(ws.Collection.GetAPIKind())+"-sample"), + Comments: nil, Markers: []string{ + "+kubebuilder:validation:Required", "Required if specifying collection. The name of the collection", "within a specific collection.namespace to reference.", }, }, { - Name: "Namespace", - Type: FieldString, - Tags: fmt.Sprintf("`json:%q`", "namespace"), - Sample: fmt.Sprintf("#namespace: %q", sampleNamespace), + Name: "Namespace", + Type: FieldString, + Tags: fmt.Sprintf("`json:%q`", "namespace,omitempty"), + Sample: fmt.Sprintf("#namespace: %q", sampleNamespace), + Comments: nil, Markers: []string{ "+kubebuilder:validation:Optional", "(Default: \"\") The namespace where the collection exists. Required only if", @@ -173,6 +175,7 @@ func (ws *WorkloadSpec) processManifests(markerTypes ...MarkerType) error { // 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()) @@ -182,12 +185,6 @@ func (ws *WorkloadSpec) processManifests(markerTypes ...MarkerType) error { return err } - ws.OwnershipRules.addOrUpdateOwnership( - manifestObject.GetAPIVersion(), - manifestObject.GetKind(), - resourceGroup, - ) - resource := ChildResource{ Name: manifestObject.GetName(), UniqueName: resourceUniqueName, diff --git a/test/cases/platform/.workloadConfig/tenancy/ns-operator-ns.yaml b/test/cases/platform/.workloadConfig/tenancy/ns-operator-ns.yaml index 895f7d10..74a69e02 100644 --- a/test/cases/platform/.workloadConfig/tenancy/ns-operator-ns.yaml +++ b/test/cases/platform/.workloadConfig/tenancy/ns-operator-ns.yaml @@ -5,4 +5,4 @@ metadata: # component belong name: tenancy-system # +operator-builder:collection:field:name=test.namespace,default=tenancy-system,type=string labels: - workload-collection: default-collection + workload-collection: default-collection # +operator-builder:collection:field:name=collectionLabel,default=default-collection,type=string