diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 95c960b..ec3a006 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -1,4 +1,4 @@ -name: tests +name: tests and lint on: pull_request: @@ -41,5 +41,7 @@ jobs: restore-keys: | ${{ runner.os }}-go- - run: git config --global url.https://${{ secrets.OCMBOT_APP_ID }}:${{ steps.generate_token.outputs.token }}@github.com/.insteadOf https://github.com/ + - name: Run lint + run: make lint - name: Run tests run: make test diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..b8430e6 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,119 @@ +run: + go: "1.21" + timeout: 10m + tests: false + allow-parallel-runners: true + skip-dirs: + - "./*/mock" + +linters-settings: + funlen: + lines: 150 + statements: 60 + staticcheck: + go: "1.21" + stylecheck: + go: "1.21" + cyclop: + max-complexity: 20 + skip-tests: true + gosec: + exclude-generated: true + lll: + line-length: 150 + misspell: + locale: US + govet: + check-shadowing: true + nilaway: + nolintlint: + allow-unused: true + require-explanation: true + require-specific: false + varnamelen: + ignore-names: + - err + - wg + - fs + - id + - vm + - ns + - ip + +issues: + max-same-issues: 0 + max-issues-per-linter: 0 + exclude-rules: + - text: "should not use dot imports|don't use an underscore in package name" + linters: + - golint + - source: "https://" + linters: + - lll + - path: pkg/defaults/ + linters: + - lll + - path: _test\.go + linters: + - goerr113 + - gocyclo + - errcheck + - gosec + - dupl + - funlen + - scopelint + - testpackage + - goconst + - godox + - path: internal/version/ + linters: + - gochecknoglobals + - path: internal/command/ + linters: + - exhaustivestruct + - lll + - wrapcheck + - source: "// .* #\\d+" + linters: + - godox + - path: test/e2e/ + linters: + - goerr113 + - gomnd + # remove this once https://github.com/golangci/golangci-lint/issues/2649 is closed + - path: / + linters: + - typecheck + +linters: + enable-all: true + disable: + - gci + - depguard + - exhaustivestruct + - golint + - interfacer + - ireturn + - maligned + - nilnil + - scopelint + - tagliatelle + - gomoddirectives + - varcheck + - nosnakecase + - structcheck + - ifshort + - deadcode + - forbidigo + - prealloc + - gochecknoinits + - exhaustruct + - goerr113 + - govet + - nonamedreturns + - varnamelen + - wrapcheck + - staticcheck + - gochecknoglobals + - paralleltest + - wsl diff --git a/Makefile b/Makefile index edef86a..472e294 100644 --- a/Makefile +++ b/Makefile @@ -53,6 +53,10 @@ fmt: ## Run go fmt against code. vet: ## Run go vet against code. go vet ./... +.PHONY: lint +lint: golangci-lint ## Run golangci-lint. + $(GOLANGCI_LINT) run + .PHONY: test test: manifests generate fmt vet envtest ## Run tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -coverprofile cover.out @@ -126,6 +130,7 @@ $(LOCALBIN): mkdir -p $(LOCALBIN) ## Tool Binaries +GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint KUSTOMIZE ?= $(LOCALBIN)/kustomize CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen ENVTEST ?= $(LOCALBIN)/setup-envtest @@ -133,6 +138,7 @@ ENVTEST ?= $(LOCALBIN)/setup-envtest ## Tool Versions KUSTOMIZE_VERSION ?= v3.8.7 CONTROLLER_TOOLS_VERSION ?= v0.11.1 +GOLANGCI_LINT_VERSION ?= v1.55.2 KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh" .PHONY: kustomize @@ -154,3 +160,8 @@ $(CONTROLLER_GEN): $(LOCALBIN) envtest: $(ENVTEST) ## Download envtest-setup locally if necessary. $(ENVTEST): $(LOCALBIN) test -s $(LOCALBIN)/setup-envtest || GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest + +.PHONY: golangci-lint +golangci-lint: $(GOLANGCI_LINT) +$(GOLANGCI_LINT): $(LOCALBIN) + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s $(GOLANGCI_LINT_VERSION) diff --git a/api/v1alpha1/condition_types.go b/api/v1alpha1/condition_types.go index 72598e1..691f361 100644 --- a/api/v1alpha1/condition_types.go +++ b/api/v1alpha1/condition_types.go @@ -14,7 +14,7 @@ const ( ServiceAccountCreateOrUpdateFailedReason string = "ServiceAccountCreateOrUpdateFailed" // RBACCreateOrUpdateFailedReason indicates that the project cluster role could not be reconciled. - RBACCreateOrUpdateFailedReason string = "RBACCreateOrUpdateFailed" + RBACCreateOrUpdateFailedReason string = "RBACCreateOrUpdateFailed" //nolint:gosec // not a cred // CertificateCreateOrUpdateFailedReason indicates that the project certificate could not be reconciled. CertificateCreateOrUpdateFailedReason string = "CertificateCreateOrUpdateFailed" diff --git a/api/v1alpha1/constants.go b/api/v1alpha1/constants.go index 4c4a302..ad49ea3 100644 --- a/api/v1alpha1/constants.go +++ b/api/v1alpha1/constants.go @@ -13,5 +13,5 @@ const ( const ( // ManagedMPASSecretAnnotationKey denotes that the project controller needs to set these secrets // in the service account of the project. - ManagedMPASSecretAnnotationKey = "mpas.ocm.system/secret.dockerconfig" + ManagedMPASSecretAnnotationKey = "mpas.ocm.system/secret.dockerconfig" //nolint:gosec // not a cred ) diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go index 8ed463d..2e254dc 100644 --- a/api/v1alpha1/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -13,10 +13,10 @@ import ( ) var ( - // GroupVersion is group version used to register these objects + // GroupVersion is group version used to register these objects. GroupVersion = schema.GroupVersion{Group: "mpas.ocm.software", Version: "v1alpha1"} - // SchemeBuilder is used to add go types to the GroupVersionKind scheme + // SchemeBuilder is used to add go types to the GroupVersionKind scheme. SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} // AddToScheme adds the types in this group-version to the given scheme. diff --git a/api/v1alpha1/project_types.go b/api/v1alpha1/project_types.go index 1667a15..e8a8f80 100644 --- a/api/v1alpha1/project_types.go +++ b/api/v1alpha1/project_types.go @@ -23,7 +23,7 @@ const ( // ExistingRepositoryPolicy defines what to do in case a requested repository already exists. type ExistingRepositoryPolicy string -// ProjectSpec defines the desired state of Project +// ProjectSpec defines the desired state of Project. type ProjectSpec struct { // +required Git gcv1alpha1.RepositorySpec `json:"git"` @@ -51,7 +51,7 @@ type CommitTemplate struct { Message string } -// ProjectStatus defines the observed state of Project +// ProjectStatus defines the observed state of Project. type ProjectStatus struct { // +optional Conditions []metav1.Condition `json:"conditions,omitempty"` @@ -78,15 +78,17 @@ func (in *Project) GetServiceAccountNamespacedName() (types.NamespacedName, erro } var name, namespace string + const nameNamespaceCount = 2 for _, e := range in.Status.Inventory.Entries { split := strings.Split(e.ID, "_") - if len(split) < 2 { + if len(split) < nameNamespaceCount { return types.NamespacedName{}, fmt.Errorf("failed to split ID: %s", e.ID) } if split[len(split)-1] == "ServiceAccount" { name = split[1] namespace = split[0] + break } } @@ -96,7 +98,6 @@ func (in *Project) GetServiceAccountNamespacedName() (types.NamespacedName, erro } return types.NamespacedName{Name: name, Namespace: namespace}, nil - } // +kubebuilder:object:root=true @@ -105,7 +106,7 @@ func (in *Project) GetServiceAccountNamespacedName() (types.NamespacedName, erro // +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description="" // +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",description="" -// Project is the Schema for the projects API +// Project is the Schema for the projects API. type Project struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -135,7 +136,7 @@ func (in *Project) GetNameWithPrefix(prefix string) string { //+kubebuilder:object:root=true -// ProjectList contains a list of Project +// ProjectList contains a list of Project. type ProjectList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/config/crd/bases/mpas.ocm.software_projects.yaml b/config/crd/bases/mpas.ocm.software_projects.yaml index e6fb370..e6b3f8f 100644 --- a/config/crd/bases/mpas.ocm.software_projects.yaml +++ b/config/crd/bases/mpas.ocm.software_projects.yaml @@ -27,7 +27,7 @@ spec: name: v1alpha1 schema: openAPIV3Schema: - description: Project is the Schema for the projects API + description: Project is the Schema for the projects API. properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation @@ -42,7 +42,7 @@ spec: metadata: type: object spec: - description: ProjectSpec defines the desired state of Project + description: ProjectSpec defines the desired state of Project. properties: flux: default: @@ -143,7 +143,7 @@ spec: - git type: object status: - description: ProjectStatus defines the observed state of Project + description: ProjectStatus defines the observed state of Project. properties: conditions: items: diff --git a/controllers/get_project_in_namespace.go b/controllers/get_project_in_namespace.go index 76bbcd6..abaeed8 100644 --- a/controllers/get_project_in_namespace.go +++ b/controllers/get_project_in_namespace.go @@ -12,9 +12,7 @@ import ( "github.com/open-component-model/mpas-project-controller/api/v1alpha1" ) -var ( - errNotProjectNamespace = errors.New("not in a namespace that belongs to a project") -) +var errNotProjectNamespace = errors.New("not in a namespace that belongs to a project") // GetProjectFromObjectNamespace returns the Project from the annotation of the current namespace that an object // is in. diff --git a/controllers/project_controller.go b/controllers/project_controller.go index eb9e213..c66a0a6 100644 --- a/controllers/project_controller.go +++ b/controllers/project_controller.go @@ -6,7 +6,7 @@ package controllers import ( "context" - _ "embed" + _ "embed" // embedding "errors" "fmt" @@ -38,11 +38,11 @@ import ( ) const ( - ControllerServiceAccountName = "mpas-project-controller" + ControllerName = "mpas-project-controller" ) const ( - // Mandatory Labels + // Mandatory Labels. labelComponent = "app.kubernetes.io/component" labelCreatedBy = "app.kubernetes.io/created-by" labelManagedBy = "app.kubernetes.io/managed-by" @@ -51,7 +51,7 @@ const ( labelPartOf = "app.kubernetes.io/part-of" ) -// ProjectReconciler reconciles a Project object +// ProjectReconciler reconciles a Project object. type ProjectReconciler struct { client.Client Scheme *runtime.Scheme @@ -66,9 +66,12 @@ type ProjectReconciler struct { //+kubebuilder:rbac:groups="",resources=namespaces;serviceaccounts;secrets,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterroles;clusterrolebindings,verbs=get;list;watch +//nolint:lll // rbac comment //+kubebuilder:rbac:groups=mpas.ocm.software,resources=projects;targets;repositories;productdeployments;productdeploymentgenerators;productdeploymentpipelines,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=mpas.ocm.software,resources=subscriptions,verbs=get;list;watch +//nolint:lll // rbac comment //+kubebuilder:rbac:groups=delivery.ocm.software,resources=componentsubscriptions;componentversions;configurations;localizations,verbs=get;list;watch;create;update;patch;delete +//nolint:lll // rbac comment //+kubebuilder:rbac:groups=source.toolkit.fluxcd.io;kustomize.toolkit.fluxcd.io,resources=gitrepositories;ocirepositories;kustomizations,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=mpas.ocm.software,resources=projects/status,verbs=get;update;patch //+kubebuilder:rbac:groups=mpas.ocm.software,resources=projects/finalizers,verbs=update @@ -98,11 +101,12 @@ func (r *ProjectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ obj := &mpasv1alpha1.Project{} if err := r.Get(ctx, req.NamespacedName, obj); err != nil { if apierrors.IsNotFound(err) { - return + return ctrl.Result{}, nil } + return ctrl.Result{}, fmt.Errorf("failed to get project %s/%s: %w", req.NamespacedName.Namespace, req.NamespacedName.Name, err) } - //Initialize the patch helper with the current version of the object. + // Initialize the patch helper with the current version of the object. patchHelper := patch.NewSerialPatcher(obj, r.Client) defer func() { @@ -113,12 +117,14 @@ func (r *ProjectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ if !controllerutil.ContainsFinalizer(obj, mpasv1alpha1.ProjectFinalizer) { controllerutil.AddFinalizer(obj, mpasv1alpha1.ProjectFinalizer) + return ctrl.Result{Requeue: true}, nil } if obj.DeletionTimestamp != nil { logger.Info("project is being deleted...") - return r.finalize(ctx, obj) + + return ctrl.Result{}, r.finalize(ctx, obj) } return r.reconcile(ctx, obj, patchHelper) @@ -161,73 +167,9 @@ func (r *ProjectReconciler) reconcile(ctx context.Context, obj *mpasv1alpha1.Pro obj.Status.Inventory.DeepCopyInto(oldInventory) } - ns, err := r.reconcileNamespace(ctx, obj) - if err != nil { - logger.Error(err, "failed to reconcile namespace") - conditions.MarkStalled(obj, mpasv1alpha1.NamespaceCreateOrUpdateFailedReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.NamespaceCreateOrUpdateFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error reconciling namespace: %w", err) - } - - sa, err := r.reconcileServiceAccount(ctx, obj) - if err != nil { - logger.Error(err, "failed to reconcile service account") - conditions.MarkStalled(obj, mpasv1alpha1.ServiceAccountCreateOrUpdateFailedReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.ServiceAccountCreateOrUpdateFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error reconciling service account: %w", err) - } - - role, err := r.reconcileRole(ctx, obj) - if err != nil { - logger.Error(err, "failed to reconcile role") - conditions.MarkStalled(obj, mpasv1alpha1.RBACCreateOrUpdateFailedReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.RBACCreateOrUpdateFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error reconciling project namespace role: %w", err) - } - - roleBindings, err := r.reconcileRoleBindings(ctx, obj, sa) - if err != nil { - logger.Error(err, "failed to reconcile cluster role binding") - conditions.MarkStalled(obj, mpasv1alpha1.RBACCreateOrUpdateFailedReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.RBACCreateOrUpdateFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error reconciling role bindings: %w", err) - } - - certificate, err := r.reconcileCertificate(ctx, obj) - if err != nil { - logger.Error(err, "failed to reconcile certificate request in namespace") - conditions.MarkStalled(obj, mpasv1alpha1.CertificateCreateOrUpdateFailedReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.CertificateCreateOrUpdateFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error reconciling certificate: %w", err) - } - - repo, err := r.reconcileRepository(ctx, obj) - if err != nil { - logger.Error(err, "failed to reconcile repository") - conditions.MarkStalled(obj, mpasv1alpha1.RepositoryCreateOrUpdateFailedReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.RepositoryCreateOrUpdateFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error reconciling repository: %w", err) - } - - obj.Status.RepositoryRef = &meta.NamespacedObjectReference{ - Name: repo.GetName(), - Namespace: repo.GetNamespace(), - } - - gitRepo, err := r.reconcileFluxGitRepository(ctx, obj, repo) + objects, err := r.reconcileInventory(ctx, obj) if err != nil { - logger.Error(err, "failed to reconcile flux git repository") - conditions.MarkStalled(obj, mpasv1alpha1.FluxGitRepositoryCreateOrUpdateFailedReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.FluxGitRepositoryCreateOrUpdateFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error reconciling flux git source: %w", err) - } - - kustomizations, err := r.reconcileFluxKustomizations(ctx, obj) - if err != nil { - logger.Error(err, "failed to reconcile flux kustomizations") - conditions.MarkStalled(obj, mpasv1alpha1.FluxKustomizationsCreateOrUpdateFailedReason, err.Error()) - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.FluxKustomizationsCreateOrUpdateFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error reconciling flux kustomizations: %w", err) + return ctrl.Result{}, err } // Requeue the project only if it was previously not requeued in order to wait for resources to be created. @@ -243,29 +185,17 @@ func (r *ProjectReconciler) reconcile(ctx context.Context, obj *mpasv1alpha1.Pro return ctrl.Result{}, fmt.Errorf("failed to patch object: %w", err) } + return ctrl.Result{Requeue: true}, nil } newInventory := inventory.New() - if err := inventory.Add(newInventory, ns, sa, role, certificate, repo, gitRepo); err != nil { - logger.Error(err, "failed to add resources to inventory") + if err := inventory.Add(newInventory, objects...); err != nil { conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.ReconciliationFailedReason, err.Error()) + return ctrl.Result{}, fmt.Errorf("error adding resources to inventory: %w", err) } - for _, roleBinding := range roleBindings { - if err := inventory.Add(newInventory, roleBinding); err != nil { - logger.Error(err, "failed to add resources to inventory") - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.ReconciliationFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error adding resources to inventory: %w", err) - } - } - for _, kustomization := range kustomizations { - if err := inventory.Add(newInventory, kustomization); err != nil { - logger.Error(err, "failed to add resources to inventory") - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.ReconciliationFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error adding resources to inventory: %w", err) - } - } + obj.Status.Inventory = newInventory obj.Status.ObservedGeneration = obj.Generation @@ -274,6 +204,7 @@ func (r *ProjectReconciler) reconcile(ctx context.Context, obj *mpasv1alpha1.Pro staleObjects, err := inventory.Diff(oldInventory, newInventory) if err != nil { conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.ReconciliationFailedReason, err.Error()) + return ctrl.Result{}, fmt.Errorf("error generating inventory diff inventory: %w", err) } @@ -281,6 +212,7 @@ func (r *ProjectReconciler) reconcile(ctx context.Context, obj *mpasv1alpha1.Pro // Garbage collect stale objects, as long as prune it set to true. if err := r.prune(ctx, obj, staleObjects); err != nil { conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.ReconciliationFailedReason, err.Error()) + return ctrl.Result{}, fmt.Errorf("error pruning stale objects: %w", err) } @@ -291,6 +223,11 @@ func (r *ProjectReconciler) reconcile(ctx context.Context, obj *mpasv1alpha1.Pro return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } +func (r *ProjectReconciler) markStalled(reason string, obj *mpasv1alpha1.Project, err error) { + conditions.MarkStalled(obj, reason, err.Error()) + conditions.MarkFalse(obj, meta.ReadyCondition, reason, err.Error()) +} + func (r *ProjectReconciler) reconcileNamespace(ctx context.Context, obj *mpasv1alpha1.Project) (*corev1.Namespace, error) { name := obj.GetNameWithPrefix(r.Prefix) ns := &corev1.Namespace{ @@ -312,7 +249,6 @@ func (r *ProjectReconciler) reconcileNamespace(ctx context.Context, obj *mpasv1a return nil }) - if err != nil { return nil, fmt.Errorf("failed to create namespace: %w", err) } @@ -340,7 +276,6 @@ func (r *ProjectReconciler) reconcileServiceAccount(ctx context.Context, obj *mp return nil }) - if err != nil { return nil, fmt.Errorf("failed to create or update service account: %w", err) } @@ -395,7 +330,6 @@ func (r *ProjectReconciler) reconcileRole(ctx context.Context, obj *mpasv1alpha1 return nil }) - if err != nil { return nil, fmt.Errorf("failed to create or update role: %w", err) } @@ -403,7 +337,11 @@ func (r *ProjectReconciler) reconcileRole(ctx context.Context, obj *mpasv1alpha1 return role, nil } -func (r *ProjectReconciler) reconcileRoleBindings(ctx context.Context, obj *mpasv1alpha1.Project, sa *corev1.ServiceAccount) ([]*rbacv1.RoleBinding, error) { +func (r *ProjectReconciler) reconcileRoleBindings( + ctx context.Context, + obj *mpasv1alpha1.Project, + sa *corev1.ServiceAccount, +) ([]*rbacv1.RoleBinding, error) { name := obj.GetNameWithPrefix(r.Prefix) key := types.NamespacedName{ Name: r.ClusterRoleName, @@ -449,7 +387,6 @@ func (r *ProjectReconciler) reconcileRoleBindings(ctx context.Context, obj *mpas return nil }) - if err != nil { return nil, fmt.Errorf("failed to create or update role binding: %w", err) } @@ -559,7 +496,6 @@ func (r *ProjectReconciler) reconcileRepository(ctx context.Context, obj *mpasv1 return nil }) - if err != nil { return nil, fmt.Errorf("failed to create or update repository: %w", err) } @@ -567,7 +503,11 @@ func (r *ProjectReconciler) reconcileRepository(ctx context.Context, obj *mpasv1 return repo, nil } -func (r *ProjectReconciler) reconcileFluxGitRepository(ctx context.Context, obj *mpasv1alpha1.Project, repo *gcv1alpha1.Repository) (*sourcev1.GitRepository, error) { +func (r *ProjectReconciler) reconcileFluxGitRepository( + ctx context.Context, + obj *mpasv1alpha1.Project, + repo *gcv1alpha1.Repository, +) (*sourcev1.GitRepository, error) { name := obj.GetNameWithPrefix(r.Prefix) gitRepo := &sourcev1.GitRepository{ @@ -598,7 +538,6 @@ func (r *ProjectReconciler) reconcileFluxGitRepository(ctx context.Context, obj return nil }) - if err != nil { return nil, fmt.Errorf("failed to create or update repository: %w", err) } @@ -634,7 +573,7 @@ func (r *ProjectReconciler) reconcileFluxKustomizations(ctx context.Context, obj Name: prefixedName, Namespace: obj.GetNamespace(), } - kustomization.Spec.ServiceAccountName = ControllerServiceAccountName + kustomization.Spec.ServiceAccountName = ControllerName kustomization.Spec.TargetNamespace = prefixedName if kustomization.Labels == nil { @@ -644,7 +583,6 @@ func (r *ProjectReconciler) reconcileFluxKustomizations(ctx context.Context, obj return nil }) - if err != nil { return nil, fmt.Errorf("failed to create or update kustomization: %w", err) } @@ -655,36 +593,30 @@ func (r *ProjectReconciler) reconcileFluxKustomizations(ctx context.Context, obj return kustomizations, nil } -func (r *ProjectReconciler) finalize(ctx context.Context, obj *mpasv1alpha1.Project) (ctrl.Result, error) { +func (r *ProjectReconciler) finalize(ctx context.Context, obj *mpasv1alpha1.Project) error { logger := log.FromContext(ctx) var retErr error - if obj.Spec.Prune && - obj.Status.Inventory != nil && - obj.Status.Inventory.Entries != nil { + if obj.Spec.Prune && obj.Status.Inventory != nil && obj.Status.Inventory.Entries != nil { objects, _ := inventory.List(obj.Status.Inventory) for _, object := range objects { - if err := r.Client.Get(ctx, client.ObjectKeyFromObject(object), object); err != nil { + if err := r.Client.Delete(ctx, object); err != nil { if !apierrors.IsNotFound(err) { - logger.Error(err, "failed to get object for deletion") + logger.Error(err, "failed to delete object", "object", object) retErr = errors.Join(retErr, err) } } - - if err := r.Client.Delete(ctx, object); err != nil { - logger.Error(err, "failed to delete object", "object", object) - retErr = errors.Join(retErr, err) - } } if retErr != nil { - return ctrl.Result{}, retErr + return retErr } } // Remove our finalizer from the list and update it controllerutil.RemoveFinalizer(obj, mpasv1alpha1.ProjectFinalizer) - return ctrl.Result{}, nil + + return nil } func (r *ProjectReconciler) prune(ctx context.Context, obj *mpasv1alpha1.Project, staleObjects []*unstructured.Unstructured) error { @@ -767,6 +699,7 @@ func (r *ProjectReconciler) reconcileCertificate(ctx context.Context, obj *mpasv if _, err := controllerutil.CreateOrUpdate(ctx, r.Client, cert, func() error { // Make sure the fields are all up-to-date + const keySize = 256 cert.Spec = certmanagerv1.CertificateSpec{ DNSNames: []string{r.RegistryAddr}, SecretName: "ocm-registry-tls-certs", @@ -777,7 +710,7 @@ func (r *ProjectReconciler) reconcileCertificate(ctx context.Context, obj *mpasv }, PrivateKey: &certmanagerv1.CertificatePrivateKey{ Algorithm: certmanagerv1.ECDSAKeyAlgorithm, - Size: 256, + Size: keySize, }, } @@ -792,8 +725,85 @@ func (r *ProjectReconciler) reconcileCertificate(ctx context.Context, obj *mpasv func (r *ProjectReconciler) applyMandatoryLabels(name, component, instance string, labels map[string]string) { labels[labelComponent] = component labels[labelInstance] = instance - labels[labelCreatedBy] = "mpas-project-controller" - labels[labelManagedBy] = "mpas-project-controller" - labels[labelPartOf] = "mpas-project-controller" + labels[labelCreatedBy] = ControllerName + labels[labelManagedBy] = ControllerName + labels[labelPartOf] = ControllerName labels[labelName] = name } + +func (r *ProjectReconciler) reconcileInventory(ctx context.Context, obj *mpasv1alpha1.Project) ([]runtime.Object, error) { + var result []runtime.Object + + ns, err := r.reconcileNamespace(ctx, obj) + if err != nil { + r.markStalled(mpasv1alpha1.NamespaceCreateOrUpdateFailedReason, obj, err) + + return nil, fmt.Errorf("error reconciling namespace: %w", err) + } + + sa, err := r.reconcileServiceAccount(ctx, obj) + if err != nil { + r.markStalled(mpasv1alpha1.ServiceAccountCreateOrUpdateFailedReason, obj, err) + + return nil, fmt.Errorf("error reconciling service account: %w", err) + } + + role, err := r.reconcileRole(ctx, obj) + if err != nil { + r.markStalled(mpasv1alpha1.RBACCreateOrUpdateFailedReason, obj, err) + + return nil, fmt.Errorf("error reconciling project namespace role: %w", err) + } + + roleBindings, err := r.reconcileRoleBindings(ctx, obj, sa) + if err != nil { + r.markStalled(mpasv1alpha1.RBACCreateOrUpdateFailedReason, obj, err) + + return nil, fmt.Errorf("error reconciling role bindings: %w", err) + } + + certificate, err := r.reconcileCertificate(ctx, obj) + if err != nil { + r.markStalled(mpasv1alpha1.CertificateCreateOrUpdateFailedReason, obj, err) + + return nil, fmt.Errorf("error reconciling certificate: %w", err) + } + + repo, err := r.reconcileRepository(ctx, obj) + if err != nil { + r.markStalled(mpasv1alpha1.RepositoryCreateOrUpdateFailedReason, obj, err) + + return nil, fmt.Errorf("error reconciling repository: %w", err) + } + + obj.Status.RepositoryRef = &meta.NamespacedObjectReference{ + Name: repo.GetName(), + Namespace: repo.GetNamespace(), + } + + gitRepo, err := r.reconcileFluxGitRepository(ctx, obj, repo) + if err != nil { + r.markStalled(mpasv1alpha1.FluxGitRepositoryCreateOrUpdateFailedReason, obj, err) + + return nil, fmt.Errorf("error reconciling flux git source: %w", err) + } + + kustomizations, err := r.reconcileFluxKustomizations(ctx, obj) + if err != nil { + r.markStalled(mpasv1alpha1.FluxKustomizationsCreateOrUpdateFailedReason, obj, err) + + return nil, fmt.Errorf("error reconciling flux kustomizations: %w", err) + } + + result = append(result, ns, sa, role, certificate, repo, gitRepo) + + for _, k := range kustomizations { + result = append(result, k) + } + + for _, r := range roleBindings { + result = append(result, r) + } + + return result, nil +} diff --git a/controllers/secret_annotation_exists_predicate.go b/controllers/secret_annotation_exists_predicate.go index ba84115..c8bda0a 100644 --- a/controllers/secret_annotation_exists_predicate.go +++ b/controllers/secret_annotation_exists_predicate.go @@ -60,5 +60,6 @@ func checkAnnotation(obj client.Object) bool { } _, ok = secret.Annotations[v1alpha1.ManagedMPASSecretAnnotationKey] + return ok } diff --git a/controllers/secrets_controller.go b/controllers/secrets_controller.go index eba492b..6875573 100644 --- a/controllers/secrets_controller.go +++ b/controllers/secrets_controller.go @@ -21,7 +21,7 @@ import ( "github.com/open-component-model/mpas-project-controller/api/v1alpha1" ) -// SecretsReconciler reconciles a Secret object +// SecretsReconciler reconciles a Secret object. type SecretsReconciler struct { client.Client kuberecorder.EventRecorder @@ -101,7 +101,9 @@ func (r *SecretsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ if err := r.Get(ctx, req.NamespacedName, secret); err != nil { if apierrors.IsNotFound(err) { // make sure we don't have it in our list of image pull secrets. - return r.reconcileDelete(ctx, serviceAccount, secret) + r.reconcileDelete(ctx, serviceAccount, secret) + + return ctrl.Result{}, nil } return ctrl.Result{}, fmt.Errorf("failed to fetch secret from cluster: %w", err) @@ -109,14 +111,18 @@ func (r *SecretsReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ // reconcile delete if secret.DeletionTimestamp != nil { - return r.reconcileDelete(ctx, serviceAccount, secret) + r.reconcileDelete(ctx, serviceAccount, secret) + + return ctrl.Result{}, nil } + r.reconcileNormal(ctx, serviceAccount, secret) + // reconcile normal - return r.reconcileNormal(ctx, serviceAccount, secret) + return ctrl.Result{}, nil } -func (r *SecretsReconciler) reconcileNormal(ctx context.Context, account *corev1.ServiceAccount, secret *corev1.Secret) (ctrl.Result, error) { +func (r *SecretsReconciler) reconcileNormal(ctx context.Context, account *corev1.ServiceAccount, secret *corev1.Secret) { logger := log.FromContext(ctx) logger.Info("reconciling secret to image pull secrets.") @@ -128,26 +134,22 @@ func (r *SecretsReconciler) reconcileNormal(ctx context.Context, account *corev1 logger.Info("nothing to do, secret already added to image pull secrets") - return ctrl.Result{}, nil + return } account.ImagePullSecrets = append(account.ImagePullSecrets, corev1.LocalObjectReference{Name: secret.Name}) - - return ctrl.Result{}, nil } -func (r *SecretsReconciler) reconcileDelete(ctx context.Context, account *corev1.ServiceAccount, secret *corev1.Secret) (ctrl.Result, error) { +func (r *SecretsReconciler) reconcileDelete(ctx context.Context, account *corev1.ServiceAccount, secret *corev1.Secret) { logger := log.FromContext(ctx) if !r.containsSecret(account.ImagePullSecrets, secret.Name) { // nothing to do, secret already removed from service account logger.Info("nothing to do, secret already removed from image pull secrets") - return ctrl.Result{}, nil + return } r.deleteSecret(account, secret) - - return ctrl.Result{}, nil } func (r *SecretsReconciler) deleteSecret(account *corev1.ServiceAccount, secret *corev1.Secret) { @@ -155,6 +157,7 @@ func (r *SecretsReconciler) deleteSecret(account *corev1.ServiceAccount, secret for i := 0; i < len(pullSecrets); i++ { if pullSecrets[i].Name == secret.Name { pullSecrets = append(pullSecrets[:i], pullSecrets[i+1:]...) + break } } diff --git a/inventory/inventory.go b/inventory/inventory.go index 04bc82e..490fdd7 100644 --- a/inventory/inventory.go +++ b/inventory/inventory.go @@ -95,6 +95,7 @@ func Diff(source *mpasv1alpha1.ResourceInventory, target *mpasv1alpha1.ResourceI return entry.Version } } + return "" } @@ -124,5 +125,6 @@ func Diff(source *mpasv1alpha1.ResourceInventory, target *mpasv1alpha1.ResourceI } sort.Sort(ssa.SortableUnstructureds(objects)) + return objects, nil } diff --git a/main.go b/main.go index fab53a0..11ee1ef 100644 --- a/main.go +++ b/main.go @@ -13,6 +13,8 @@ import ( // to ensure that exec-entrypoint and run can make use of them. _ "k8s.io/client-go/plugin/pkg/client/auth" + kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" + sourcev1 "github.com/fluxcd/source-controller/api/v1" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -20,9 +22,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/healthz" "sigs.k8s.io/controller-runtime/pkg/log/zap" - kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1" - sourcev1 "github.com/fluxcd/source-controller/api/v1" - gcv1alpha1 "github.com/open-component-model/git-controller/apis/mpas/v1alpha1" mpasv1alpha1 "github.com/open-component-model/mpas-project-controller/api/v1alpha1" "github.com/open-component-model/mpas-project-controller/controllers" @@ -60,19 +59,59 @@ func main() { certificateIssuerName string ) - flag.StringVar(&certificateIssuerName, "certificate-issuer-name", "mpas-certificate-issuer", "The name of the ClusterIssuer to request certificates from. By default this is created by the MPAS Bootstrap command.") - flag.StringVar(®istryAddress, "registry-address", "registry.ocm-system.svc.cluster.local", "The address of the internal registry. This is used for the certificate DNS names.") + flag.StringVar( + &certificateIssuerName, + "certificate-issuer-name", + "mpas-certificate-issuer", + "The name of the ClusterIssuer to request certificates from. By default this is created by the MPAS Bootstrap command.", + ) + flag.StringVar( + ®istryAddress, + "registry-address", + "registry.ocm-system.svc.cluster.local", + "The address of the internal registry. This is used for the certificate DNS names.", + ) flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.") flag.BoolVar(&enableLeaderElection, "leader-elect", false, "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") - flag.StringVar(&clusterRoleName, "cluster-role-name", "mpas-projects-clusterrole", "The name of the cluster role to use for project ServiceAccounts.") - flag.StringVar(&prefix, "prefix", "mpas", "The prefix to use for all resources and Git repositories created by the controller.") - flag.StringVar(&defaultCommitName, "default-commit-name", "MPAS System", "The name to use for automated commits if one is not provided in the Project spec.") - flag.StringVar(&defaultCommitEmail, "default-commit-email", "automated@ocm.software", "The email address to use for automated commits if one is not provided in the Project spec.") - flag.StringVar(&defaultCommitMessage, "default-commit-message", "Automated commit by MPAS Project Controller", "The commit message to use for automated commits if one is not provided in the Project spec.") - flag.StringVar(&defaultNamespace, "default-namespace", "mpas-system", "The namespace in which this controller is running in. This namespace is used to locate Project objects.") + flag.StringVar( + &clusterRoleName, + "cluster-role-name", + "mpas-projects-clusterrole", + "The name of the cluster role to use for project ServiceAccounts.", + ) + flag.StringVar( + &prefix, + "prefix", + "mpas", + "The prefix to use for all resources and Git repositories created by the controller.", + ) + flag.StringVar( + &defaultCommitName, + "default-commit-name", + "MPAS System", + "The name to use for automated commits if one is not provided in the Project spec.", + ) + flag.StringVar( + &defaultCommitEmail, + "default-commit-email", + "automated@ocm.software", + "The email address to use for automated commits if one is not provided in the Project spec.", + ) + flag.StringVar( + &defaultCommitMessage, + "default-commit-message", + "Automated commit by MPAS Project Controller", + "The commit message to use for automated commits if one is not provided in the Project spec.", + ) + flag.StringVar( + &defaultNamespace, + "default-namespace", + "mpas-system", + "The namespace in which this controller is running in. This namespace is used to locate Project objects.", + ) opts := zap.Options{ Development: true, @@ -82,10 +121,11 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + const metricsServerPort = 9443 mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, - Port: 9443, + Port: metricsServerPort, HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "bccfd20b.ocm.software",