From b0d102a435c3fd41831818a794259a2acf45947b Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Wed, 29 Nov 2023 11:44:34 +0100 Subject: [PATCH] feat: applying lint and fixing the lint issues --- .github/workflows/tests.yaml | 4 +- .golangci.yaml | 121 ++++++++ Makefile | 11 + api/v1alpha1/condition_types.go | 2 +- api/v1alpha1/constants.go | 10 + api/v1alpha1/groupversion_info.go | 4 +- api/v1alpha1/productdeployment_types.go | 4 +- .../productdeploymentgenerator_types.go | 12 +- .../productdeploymentpipeline_types.go | 9 +- api/v1alpha1/productdescription_types.go | 8 +- api/v1alpha1/target_types.go | 8 +- api/v1alpha1/validation_types.go | 6 +- ....software_productdeploymentgenerators.yaml | 6 +- ...m.software_productdeploymentpipelines.yaml | 6 +- .../mpas.ocm.software_productdeployments.yaml | 2 +- ...mpas.ocm.software_productdescriptions.yaml | 6 +- .../crd/bases/mpas.ocm.software_targets.yaml | 6 +- .../bases/mpas.ocm.software_validations.yaml | 4 +- .../fetch_git_repository_from_project.go | 4 +- controllers/productdeployment_controller.go | 70 ++--- .../productdeploymentgenerator_controller.go | 261 +++++++++++------- .../productdeploymentpipeline_controller.go | 38 +-- .../productdeploymentpipeline_scheduler.go | 7 +- controllers/select_target.go | 10 +- controllers/target_controller.go | 198 +++++++------ controllers/validation_controller.go | 246 ++++++++++------- internal/cue/cue.go | 101 ++++--- internal/cue/cue_test.go | 24 +- main.go | 23 +- pkg/ocm/ocm.go | 21 +- pkg/validators/gitea/validator.go | 9 +- pkg/validators/github/validator.go | 11 +- 32 files changed, 802 insertions(+), 450 deletions(-) create mode 100644 .golangci.yaml create mode 100644 api/v1alpha1/constants.go diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 363cd76..1886286 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -1,4 +1,4 @@ -name: tests +name: tests and lint on: pull_request: @@ -32,5 +32,7 @@ jobs: key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} restore-keys: | ${{ runner.os }}-go- + - 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..3703414 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,121 @@ +run: + go: "1.21" + timeout: 10m + tests: false + allow-parallel-runners: true + skip-dirs: + - "./*/mock" + skip-files: + - "pkg/ocm/fakes/ocm.go" + +linters-settings: + funlen: + lines: 150 + statements: 70 + 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: false + 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 468f9f1..8b567d9 100644 --- a/Makefile +++ b/Makefile @@ -57,6 +57,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 ./... $(GO_TEST_ARGS) -coverprofile cover.out @@ -133,10 +137,12 @@ $(LOCALBIN): KUSTOMIZE ?= $(LOCALBIN)/kustomize CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen ENVTEST ?= $(LOCALBIN)/setup-envtest +GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint ## 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 @@ -158,3 +164,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 a150f6f..040e743 100644 --- a/api/v1alpha1/condition_types.go +++ b/api/v1alpha1/condition_types.go @@ -50,7 +50,7 @@ const ( // CreateSnapshotFailedReason is used when we fail to create an ocm-controller.Snapshot object in the cluster. CreateSnapshotFailedReason = "CreateSnapshotFailed" - // CommitTemplateEmptyReason is used when a the commit template is not set. + // CommitTemplateEmptyReason is used when a commit template is not set. CommitTemplateEmptyReason = "CommitTemplateEmpty" // CreateComponentVersionFailedReason is used when we fail to create an ocm-controller.ComponentVersion object in the cluster. diff --git a/api/v1alpha1/constants.go b/api/v1alpha1/constants.go new file mode 100644 index 0000000..4f85e5f --- /dev/null +++ b/api/v1alpha1/constants.go @@ -0,0 +1,10 @@ +package v1alpha1 + +const ( + // LevelDebug defines the debug level logs. + LevelDebug = 4 +) + +const ( + WritePermissions = 0o777 +) 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/productdeployment_types.go b/api/v1alpha1/productdeployment_types.go index a7f3f4d..9c8dc36 100644 --- a/api/v1alpha1/productdeployment_types.go +++ b/api/v1alpha1/productdeployment_types.go @@ -113,7 +113,7 @@ func (in *ProductDeployment) SetObservedGeneration(v int64) { //+kubebuilder:object:root=true //+kubebuilder:subresource:status -// ProductDeployment is the Schema for the productdeployments API +// ProductDeployment is the Schema for the productdeployments API. type ProductDeployment struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -124,7 +124,7 @@ type ProductDeployment struct { //+kubebuilder:object:root=true -// ProductDeploymentList contains a list of ProductDeployment +// ProductDeploymentList contains a list of ProductDeployment. type ProductDeploymentList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/productdeploymentgenerator_types.go b/api/v1alpha1/productdeploymentgenerator_types.go index c376055..d8b3304 100644 --- a/api/v1alpha1/productdeploymentgenerator_types.go +++ b/api/v1alpha1/productdeploymentgenerator_types.go @@ -11,7 +11,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// ProductDeploymentGeneratorSpec defines the desired state of ProductDeploymentGenerator +// ProductDeploymentGeneratorSpec defines the desired state of ProductDeploymentGenerator. type ProductDeploymentGeneratorSpec struct { // Interval is the reconciliation interval, i.e. at what interval shall a reconciliation happen. // This is used to requeue objects for reconciliation in case the related subscription hasn't been finished yet. @@ -31,7 +31,7 @@ type ProductDeploymentGeneratorSpec struct { ServiceAccountName string `json:"serviceAccountName"` } -// ProductDeploymentGeneratorStatus defines the observed state of ProductDeploymentGenerator +// ProductDeploymentGeneratorStatus defines the observed state of ProductDeploymentGenerator. type ProductDeploymentGeneratorStatus struct { // ObservedGeneration is the last reconciled generation. // +optional @@ -53,12 +53,12 @@ type ProductDeploymentGeneratorStatus struct { LastReconciledVersion string `json:"lastReconciledVersion,omitempty"` } -// GetSnapshotDigest returns the latest snapshot digest for the localization +// GetSnapshotDigest returns the latest snapshot digest for the localization. func (in ProductDeploymentGenerator) GetSnapshotDigest() string { return in.Status.LatestSnapshotDigest } -// GetSnapshotName returns the key for the snapshot produced by the Localization +// GetSnapshotName returns the key for the snapshot produced by the Localization. func (in ProductDeploymentGenerator) GetSnapshotName() string { return in.Status.SnapshotName } @@ -93,7 +93,7 @@ func (in *ProductDeploymentGenerator) SetObservedGeneration(v int64) { //+kubebuilder:object:root=true //+kubebuilder:subresource:status -// ProductDeploymentGenerator is the Schema for the productdeploymentgenerators API +// ProductDeploymentGenerator is the Schema for the productdeploymentgenerators API. type ProductDeploymentGenerator struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -104,7 +104,7 @@ type ProductDeploymentGenerator struct { //+kubebuilder:object:root=true -// ProductDeploymentGeneratorList contains a list of ProductDeploymentGenerator +// ProductDeploymentGeneratorList contains a list of ProductDeploymentGenerator. type ProductDeploymentGeneratorList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/productdeploymentpipeline_types.go b/api/v1alpha1/productdeploymentpipeline_types.go index 16dbd1e..eecbb21 100644 --- a/api/v1alpha1/productdeploymentpipeline_types.go +++ b/api/v1alpha1/productdeploymentpipeline_types.go @@ -9,7 +9,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// ProductDeploymentPipelineSpec defines the desired state of ProductDeploymentPipeline +// ProductDeploymentPipelineSpec defines the desired state of ProductDeploymentPipeline. type ProductDeploymentPipelineSpec struct { // ComponentVersionRef is the name of the generated component version object. // +required @@ -29,7 +29,7 @@ type ProductDeploymentPipelineSpec struct { TargetRef meta.NamespacedObjectReference `json:"targetRef"` } -// ProductDeploymentPipelineStatus defines the observed state of ProductDeploymentPipeline +// ProductDeploymentPipelineStatus defines the observed state of ProductDeploymentPipeline. type ProductDeploymentPipelineStatus struct { // ObservedGeneration is the last reconciled generation. // +optional @@ -84,13 +84,14 @@ func (in *ProductDeploymentPipeline) Equals(spec ProductDeploymentPipelineSpec) case in.Spec.ConfigMapRef != spec.ConfigMapRef: return false } + return true } //+kubebuilder:object:root=true //+kubebuilder:subresource:status -// ProductDeploymentPipeline is the Schema for the productdeploymentpipelines API +// ProductDeploymentPipeline is the Schema for the productdeploymentpipelines API. type ProductDeploymentPipeline struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -101,7 +102,7 @@ type ProductDeploymentPipeline struct { //+kubebuilder:object:root=true -// ProductDeploymentPipelineList contains a list of ProductDeploymentPipeline +// ProductDeploymentPipelineList contains a list of ProductDeploymentPipeline. type ProductDeploymentPipelineList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/productdescription_types.go b/api/v1alpha1/productdescription_types.go index 0858d9f..70723ea 100644 --- a/api/v1alpha1/productdescription_types.go +++ b/api/v1alpha1/productdescription_types.go @@ -8,7 +8,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// ProductDescriptionSpec defines the desired state of ProductDescription +// ProductDescriptionSpec defines the desired state of ProductDescription. type ProductDescriptionSpec struct { // +required Description string `json:"description"` @@ -19,7 +19,7 @@ type ProductDescriptionSpec struct { TargetRoles []TargetRoles `json:"targetRoles,omitempty"` } -// ProductDescriptionStatus defines the observed state of ProductDescription +// ProductDescriptionStatus defines the observed state of ProductDescription. type ProductDescriptionStatus struct{} // TargetRoles defines a target role with a name. @@ -54,7 +54,7 @@ type DescriptionConfiguration struct { //+kubebuilder:object:root=true //+kubebuilder:subresource:status -// ProductDescription is the Schema for the productdescriptions API +// ProductDescription is the Schema for the productdescriptions API. type ProductDescription struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -65,7 +65,7 @@ type ProductDescription struct { //+kubebuilder:object:root=true -// ProductDescriptionList contains a list of ProductDescription +// ProductDescriptionList contains a list of ProductDescription. type ProductDescriptionList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/target_types.go b/api/v1alpha1/target_types.go index ec26a6d..f5af1e0 100644 --- a/api/v1alpha1/target_types.go +++ b/api/v1alpha1/target_types.go @@ -23,7 +23,7 @@ var ( OCIRepository TargetType = "ocirepository" ) -// TargetSpec defines the desired state of Target +// TargetSpec defines the desired state of Target. type TargetSpec struct { // +required Type TargetType `json:"type"` @@ -47,7 +47,7 @@ type TargetSpec struct { SecretsSelector *metav1.LabelSelector `json:"selector,omitempty"` } -// TargetStatus defines the observed state of Target +// TargetStatus defines the observed state of Target. type TargetStatus struct { // ObservedGeneration is the last reconciled generation. // +optional @@ -78,7 +78,7 @@ func (in Target) GetRequeueAfter() time.Duration { //+kubebuilder:object:root=true //+kubebuilder:subresource:status -// Target is the Schema for the targets API +// Target is the Schema for the targets API. type Target struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -89,7 +89,7 @@ type Target struct { //+kubebuilder:object:root=true -// TargetList contains a list of Target +// TargetList contains a list of Target. type TargetList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/api/v1alpha1/validation_types.go b/api/v1alpha1/validation_types.go index 3d51486..3976d5e 100644 --- a/api/v1alpha1/validation_types.go +++ b/api/v1alpha1/validation_types.go @@ -26,7 +26,7 @@ type ValidationSpec struct { SyncRef meta.NamespacedObjectReference `json:"syncRef"` } -// ValidationStatus defines the observed state of Validation +// ValidationStatus defines the observed state of Validation. type ValidationStatus struct { // ObservedGeneration is the last reconciled generation. // +optional @@ -77,7 +77,7 @@ func (in Validation) GetRequeueAfter() time.Duration { //+kubebuilder:object:root=true //+kubebuilder:subresource:status -// Validation is the Schema for the validations API +// Validation is the Schema for the validations API. type Validation struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -88,7 +88,7 @@ type Validation struct { //+kubebuilder:object:root=true -// ValidationList contains a list of Validation +// ValidationList contains a list of Validation. type ValidationList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/config/crd/bases/mpas.ocm.software_productdeploymentgenerators.yaml b/config/crd/bases/mpas.ocm.software_productdeploymentgenerators.yaml index 83f7819..c88ceb5 100644 --- a/config/crd/bases/mpas.ocm.software_productdeploymentgenerators.yaml +++ b/config/crd/bases/mpas.ocm.software_productdeploymentgenerators.yaml @@ -19,7 +19,7 @@ spec: schema: openAPIV3Schema: description: ProductDeploymentGenerator is the Schema for the productdeploymentgenerators - API + API. properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation @@ -35,7 +35,7 @@ spec: type: object spec: description: ProductDeploymentGeneratorSpec defines the desired state - of ProductDeploymentGenerator + of ProductDeploymentGenerator. properties: interval: description: Interval is the reconciliation interval, i.e. at what @@ -78,7 +78,7 @@ spec: type: object status: description: ProductDeploymentGeneratorStatus defines the observed state - of ProductDeploymentGenerator + of ProductDeploymentGenerator. properties: conditions: items: diff --git a/config/crd/bases/mpas.ocm.software_productdeploymentpipelines.yaml b/config/crd/bases/mpas.ocm.software_productdeploymentpipelines.yaml index 7169ae3..24ebe85 100644 --- a/config/crd/bases/mpas.ocm.software_productdeploymentpipelines.yaml +++ b/config/crd/bases/mpas.ocm.software_productdeploymentpipelines.yaml @@ -19,7 +19,7 @@ spec: schema: openAPIV3Schema: description: ProductDeploymentPipeline is the Schema for the productdeploymentpipelines - API + API. properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation @@ -35,7 +35,7 @@ spec: type: object spec: description: ProductDeploymentPipelineSpec defines the desired state of - ProductDeploymentPipeline + ProductDeploymentPipeline. properties: componentVersionRef: description: ComponentVersionRef is the name of the generated component @@ -273,7 +273,7 @@ spec: type: object status: description: ProductDeploymentPipelineStatus defines the observed state - of ProductDeploymentPipeline + of ProductDeploymentPipeline. properties: conditions: items: diff --git a/config/crd/bases/mpas.ocm.software_productdeployments.yaml b/config/crd/bases/mpas.ocm.software_productdeployments.yaml index fcc5d0a..3ebdf0c 100644 --- a/config/crd/bases/mpas.ocm.software_productdeployments.yaml +++ b/config/crd/bases/mpas.ocm.software_productdeployments.yaml @@ -18,7 +18,7 @@ spec: - name: v1alpha1 schema: openAPIV3Schema: - description: ProductDeployment is the Schema for the productdeployments API + description: ProductDeployment is the Schema for the productdeployments API. properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation diff --git a/config/crd/bases/mpas.ocm.software_productdescriptions.yaml b/config/crd/bases/mpas.ocm.software_productdescriptions.yaml index ed8b189..199eec9 100644 --- a/config/crd/bases/mpas.ocm.software_productdescriptions.yaml +++ b/config/crd/bases/mpas.ocm.software_productdescriptions.yaml @@ -19,7 +19,7 @@ spec: schema: openAPIV3Schema: description: ProductDescription is the Schema for the productdescriptions - API + API. properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation @@ -34,7 +34,7 @@ spec: metadata: type: object spec: - description: ProductDescriptionSpec defines the desired state of ProductDescription + description: ProductDescriptionSpec defines the desired state of ProductDescription. properties: description: type: string @@ -316,7 +316,7 @@ spec: - pipelines type: object status: - description: ProductDescriptionStatus defines the observed state of ProductDescription + description: ProductDescriptionStatus defines the observed state of ProductDescription. type: object type: object served: true diff --git a/config/crd/bases/mpas.ocm.software_targets.yaml b/config/crd/bases/mpas.ocm.software_targets.yaml index 3b1e426..d107384 100644 --- a/config/crd/bases/mpas.ocm.software_targets.yaml +++ b/config/crd/bases/mpas.ocm.software_targets.yaml @@ -18,7 +18,7 @@ spec: - name: v1alpha1 schema: openAPIV3Schema: - description: Target is the Schema for the targets API + description: Target is the Schema for the targets API. properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation @@ -33,7 +33,7 @@ spec: metadata: type: object spec: - description: TargetSpec defines the desired state of Target + description: TargetSpec defines the desired state of Target. properties: access: x-kubernetes-preserve-unknown-fields: true @@ -102,7 +102,7 @@ spec: - type type: object status: - description: TargetStatus defines the observed state of Target + description: TargetStatus defines the observed state of Target. properties: conditions: items: diff --git a/config/crd/bases/mpas.ocm.software_validations.yaml b/config/crd/bases/mpas.ocm.software_validations.yaml index c9d71a1..d5f426d 100644 --- a/config/crd/bases/mpas.ocm.software_validations.yaml +++ b/config/crd/bases/mpas.ocm.software_validations.yaml @@ -18,7 +18,7 @@ spec: - name: v1alpha1 schema: openAPIV3Schema: - description: Validation is the Schema for the validations API + description: Validation is the Schema for the validations API. properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation @@ -66,7 +66,7 @@ spec: - syncRef type: object status: - description: ValidationStatus defines the observed state of Validation + description: ValidationStatus defines the observed state of Validation. properties: conditions: items: diff --git a/controllers/fetch_git_repository_from_project.go b/controllers/fetch_git_repository_from_project.go index 735fe6d..f3d7091 100644 --- a/controllers/fetch_git_repository_from_project.go +++ b/controllers/fetch_git_repository_from_project.go @@ -19,13 +19,15 @@ func FetchGitRepositoryFromProjectInventory(project *projectv1.Project) (string, var repoName, repoNamespace string for _, e := range project.Status.Inventory.Entries { split := strings.Split(e.ID, "_") - if len(split) < 2 { + splitLength := 2 + if len(split) < splitLength { return "", "", fmt.Errorf("failed to split ID: %s", e.ID) } if split[len(split)-1] == v1.GitRepositoryKind { repoName = split[1] repoNamespace = split[0] + break } } diff --git a/controllers/productdeployment_controller.go b/controllers/productdeployment_controller.go index 47290bb..625bb91 100644 --- a/controllers/productdeployment_controller.go +++ b/controllers/productdeployment_controller.go @@ -6,11 +6,10 @@ package controllers import ( "context" - "crypto/sha1" + "crypto/sha1" //nolint:gosec // good enough for our purposes "encoding/hex" "errors" "fmt" - "time" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" @@ -39,7 +38,7 @@ const ( controllerMetadataKey = ".metadata.controller" ) -// ProductDeploymentReconciler reconciles a ProductDeployment object +// ProductDeploymentReconciler reconciles a ProductDeployment object. type ProductDeploymentReconciler struct { client.Client Scheme *runtime.Scheme @@ -66,7 +65,7 @@ func (r *ProductDeploymentReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (*ProductDeploymentReconciler) indexConfigMap(mgr ctrl.Manager) error { - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &corev1.ConfigMap{}, controllerMetadataKey, func(rawObj client.Object) []string { + return mgr.GetFieldIndexer().IndexField(context.Background(), &corev1.ConfigMap{}, controllerMetadataKey, func(rawObj client.Object) []string { cfg, ok := rawObj.(*corev1.ConfigMap) if !ok { return nil @@ -81,33 +80,33 @@ func (*ProductDeploymentReconciler) indexConfigMap(mgr ctrl.Manager) error { if !ok { return nil } + return []string{owner} - }); err != nil { - return err - } - return nil + }) } func (*ProductDeploymentReconciler) indexProductDeploymentPipeline(mgr ctrl.Manager) error { - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &v1alpha1.ProductDeploymentPipeline{}, controllerMetadataKey, func(rawObj client.Object) []string { - pipeline, ok := rawObj.(*v1alpha1.ProductDeploymentPipeline) - if !ok { - return nil - } - owner := metav1.GetControllerOf(pipeline) - if owner == nil { - return nil - } + return mgr.GetFieldIndexer().IndexField( + context.Background(), + &v1alpha1.ProductDeploymentPipeline{}, + controllerMetadataKey, + func(rawObj client.Object) []string { + pipeline, ok := rawObj.(*v1alpha1.ProductDeploymentPipeline) + if !ok { + return nil + } + owner := metav1.GetControllerOf(pipeline) + if owner == nil { + return nil + } - if owner.APIVersion != v1alpha1.GroupVersion.String() || owner.Kind != v1alpha1.ProductDeploymentKind { - return nil - } + if owner.APIVersion != v1alpha1.GroupVersion.String() || owner.Kind != v1alpha1.ProductDeploymentKind { + return nil + } - return []string{owner.Name} - }); err != nil { - return err - } - return nil + return []string{owner.Name} + }, + ) } //+kubebuilder:rbac:groups=mpas.ocm.software,resources=productdeployments,verbs=get;list;watch;create;update;patch;delete @@ -121,7 +120,7 @@ func (*ProductDeploymentReconciler) indexProductDeploymentPipeline(mgr ctrl.Mana func (r *ProductDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) { logger := log.FromContext(ctx) - logger.V(4).Info("starting reconcile loop for product deployment") + logger.V(v1alpha1.LevelDebug).Info("starting reconcile loop for product deployment") obj := &v1alpha1.ProductDeployment{} if err := r.Get(ctx, req.NamespacedName, obj); err != nil { @@ -184,6 +183,7 @@ func (r *ProductDeploymentReconciler) reconcile(ctx context.Context, obj *v1alph configName, configUpdated, err := r.createOrUpdatedValuesConfigMap(ctx, obj, project) if err != nil { status.MarkNotReady(r.EventRecorder, obj, v1alpha1.CreateConfigMapFailedReason, err.Error()) + return ctrl.Result{}, fmt.Errorf("failed to validate config: %w", err) } @@ -200,6 +200,7 @@ func (r *ProductDeploymentReconciler) reconcile(ctx context.Context, obj *v1alph obj.Status.ActivePipelines = nil for _, ep := range existingPipelines.Items { + ep := ep alreadyCreatedPipelines[ep.Name] = struct{}{} if !conditions.IsTrue(&ep, meta.ReadyCondition) { @@ -251,12 +252,13 @@ func (r *ProductDeploymentReconciler) reconcile(ctx context.Context, obj *v1alph obj.Status.ActivePipelines = append(obj.Status.ActivePipelines, pobj.Name) - logger.V(4).Info("pipeline object successfully created", "name", pobj.Name) + logger.V(v1alpha1.LevelDebug).Info("pipeline object successfully created", "name", pobj.Name) } logger.Info("all pipelines handled successfully") status.MarkReady(r.EventRecorder, obj, "Reconciliation success") + //nolint:godox // yep // TODO: do something with failed and successful pipelines return ctrl.Result{RequeueAfter: jitter.JitteredIntervalDuration(obj.GetRequeueAfter())}, nil } @@ -265,7 +267,11 @@ func (r *ProductDeploymentReconciler) reconcile(ctx context.Context, obj *v1alph // contains the values.yaml file. // It retrieves the existing config.cue, performs validation and generate a yaml representation of the values that is used // to create the configmap. -func (r *ProductDeploymentReconciler) createOrUpdatedValuesConfigMap(ctx context.Context, obj *v1alpha1.ProductDeployment, project *projectv1.Project) (string, bool, error) { +func (r *ProductDeploymentReconciler) createOrUpdatedValuesConfigMap( + ctx context.Context, + obj *v1alpha1.ProductDeployment, + project *projectv1.Project, +) (string, bool, error) { config, err := fetchExistingConfigFile(ctx, r.Client, obj.Name, project) if err != nil { return "", false, fmt.Errorf("failed to retrieve config: %w", err) @@ -311,12 +317,12 @@ func (r *ProductDeploymentReconciler) createOrUpdateComponentVersion(ctx context }, Spec: ocmv1alpha1.ComponentVersionSpec{ // Note: The interval here doesn't matter because we always pin to a specific version anyway. - Interval: metav1.Duration{Duration: 10 * time.Minute}, + Interval: metav1.Duration{Duration: defaultPipelineRequeue}, Component: obj.Spec.Component.Name, Repository: ocmv1alpha1.Repository{ URL: obj.Spec.Component.Registry.URL, }, - Verify: nil, // TODO: think about this + Verify: nil, References: ocmv1alpha1.ReferencesConfig{ Expand: true, }, @@ -384,16 +390,18 @@ func (r *ProductDeploymentReconciler) generateConfigMap(ctx context.Context, obj // garbage collect old configmaps var errf error for _, cm := range existingMaps.Items { + cm := cm err := r.Client.Delete(ctx, &cm) if err != nil { errf = errors.Join(errf, err) } } + return configName, true, errf } func hashValues(values string) string { - h := sha1.New() + h := sha1.New() //nolint:gosec // good enough for our purposes h.Write([]byte(values)) return hex.EncodeToString(h.Sum(nil))[:8] diff --git a/controllers/productdeploymentgenerator_controller.go b/controllers/productdeploymentgenerator_controller.go index 50741f5..8cd3f4d 100644 --- a/controllers/productdeploymentgenerator_controller.go +++ b/controllers/productdeploymentgenerator_controller.go @@ -7,7 +7,7 @@ package controllers import ( "bytes" "context" - "crypto/sha1" + "crypto/sha1" //nolint:gosec // good enough for our purposes "encoding/hex" "errors" "fmt" @@ -74,10 +74,11 @@ const ( ) var ( - unschedulableError = errors.New("pipeline cannot be scheduled as it doesn't define any target scopes") + errUnschedulable = errors.New("pipeline cannot be scheduled as it doesn't define any target scopes") + longRequeue = 30 * time.Second ) -// ProductDeploymentGeneratorReconciler reconciles a ProductDeploymentGenerator object +// ProductDeploymentGeneratorReconciler reconciles a ProductDeploymentGenerator object. type ProductDeploymentGeneratorReconciler struct { client.Client Scheme *runtime.Scheme @@ -91,11 +92,16 @@ type ProductDeploymentGeneratorReconciler struct { // SetupWithManager sets up the controller with the Manager. func (r *ProductDeploymentGeneratorReconciler) SetupWithManager(mgr ctrl.Manager) error { if err := mgr.GetFieldIndexer().IndexField(context.TODO(), &v1alpha1.ProductDeploymentGenerator{}, sourceKey, func(rawObj client.Object) []string { - generator := rawObj.(*v1alpha1.ProductDeploymentGenerator) - var ns = generator.Spec.SubscriptionRef.Namespace + generator, ok := rawObj.(*v1alpha1.ProductDeploymentGenerator) + if !ok { + return nil + } + + ns := generator.Spec.SubscriptionRef.Namespace if ns == "" { ns = generator.GetNamespace() } + return []string{fmt.Sprintf("%s/%s", ns, generator.Spec.SubscriptionRef.Name)} }); err != nil { return fmt.Errorf("failed setting index fields: %w", err) @@ -132,7 +138,7 @@ func (r *ProductDeploymentGeneratorReconciler) SetupWithManager(mgr ctrl.Manager func (r *ProductDeploymentGeneratorReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, err error) { logger := log.FromContext(ctx) - logger.V(4).Info("starting product deployment generator loop") + logger.V(v1alpha1.LevelDebug).Info("starting product deployment generator loop") obj := &v1alpha1.ProductDeploymentGenerator{} if err := r.Get(ctx, req.NamespacedName, obj); err != nil { @@ -166,9 +172,7 @@ func (r *ProductDeploymentGeneratorReconciler) Reconcile(ctx context.Context, re } func (r *ProductDeploymentGeneratorReconciler) reconcile(ctx context.Context, obj *v1alpha1.ProductDeploymentGenerator) (_ ctrl.Result, err error) { - subscription := &replicationv1.ComponentSubscription{} - if err := r.Get(ctx, types.NamespacedName{ Name: obj.Spec.SubscriptionRef.Name, Namespace: obj.Spec.SubscriptionRef.Namespace, @@ -178,32 +182,14 @@ func (r *ProductDeploymentGeneratorReconciler) reconcile(ctx context.Context, ob return ctrl.Result{}, fmt.Errorf("failed to find subscription object: %w", err) } - if !conditions.IsReady(subscription) { - status.MarkNotReady(r.EventRecorder, obj, v1alpha1.ComponentSubscriptionNotReadyReason, "component subscription not ready yet") - - return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil + ok, err := r.shouldRun(obj, subscription) + if err != nil { + return ctrl.Result{}, err } + if !ok { + status.MarkReady(r.EventRecorder, obj, "Reconciliation success") - if obj.Status.LastReconciledVersion != "" { - lastReconciledGeneratorVersion, err := semver.NewVersion(obj.Status.LastReconciledVersion) - if err != nil { - status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SemverParseFailedReason, err.Error()) - - return ctrl.Result{}, fmt.Errorf("failed to parse last reconciled version: %w", err) - } - - lastReconciledSubscription, err := semver.NewVersion(subscription.Status.LastAppliedVersion) - if err != nil { - status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SemverParseFailedReason, err.Error()) - - return ctrl.Result{}, fmt.Errorf("failed to parse last reconciled version: %w", err) - } - - if lastReconciledSubscription.Equal(lastReconciledGeneratorVersion) || lastReconciledSubscription.LessThan(lastReconciledGeneratorVersion) { - status.MarkReady(r.EventRecorder, obj, "Reconciliation success") - - return ctrl.Result{}, nil - } + return ctrl.Result{}, nil } project, err := GetProjectFromObjectNamespace(ctx, r.Client, obj, r.MpasSystemNamespace) @@ -241,6 +227,7 @@ func (r *ProductDeploymentGeneratorReconciler) reconcile(ctx context.Context, ob return ctrl.Result{}, fmt.Errorf("failed to authenticate using service account: %w", err) } + defer func() { if cerr := cv.Close(); cerr != nil { err = errors.Join(err, cerr) @@ -276,29 +263,98 @@ func (r *ProductDeploymentGeneratorReconciler) reconcile(ctx context.Context, ob } }() - // Create top-level product folder. - if err := os.Mkdir(filepath.Join(dir, obj.Name), 0o777); err != nil { + sync, values, err := r.createSync(ctx, obj, *prodDesc, component, dir, cv, project) + if err != nil { + return ctrl.Result{}, err + } + + if sync == nil { + return ctrl.Result{}, nil + } + + validationSchema, err := values.Format() + if err != nil { + status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SchemaGenerationFailedReason, err.Error()) + + return ctrl.Result{}, fmt.Errorf("failed to create sync request: %w", err) + } + + hashedVersion := r.hashComponentVersion(component.Version) + + validation := &v1alpha1.Validation{ + ObjectMeta: metav1.ObjectMeta{ + Name: obj.Name + "-validation-" + hashedVersion, + Namespace: obj.Namespace, + Annotations: map[string]string{ + componentVersionAnnotationKey: component.Version, + componentNameAnnotationKey: component.Name, + }, + }, + Spec: v1alpha1.ValidationSpec{ + Schema: validationSchema, + ServiceAccountName: obj.Spec.ServiceAccountName, + Interval: metav1.Duration{Duration: longRequeue}, + SyncRef: meta.NamespacedObjectReference{ + Name: sync.Name, + Namespace: sync.Namespace, + }, + }, + } + + if _, err := ctrl.CreateOrUpdate(ctx, r.Client, validation, func() error { + if validation.ObjectMeta.CreationTimestamp.IsZero() { + if err := controllerutil.SetOwnerReference(obj, validation, r.Scheme); err != nil { + return fmt.Errorf("failed to set owner to validation object: %w", err) + } + } + + return nil + }); err != nil { + status.MarkNotReady(r.EventRecorder, obj, v1alpha1.CreateValidationFailedReason, err.Error()) + + return ctrl.Result{}, fmt.Errorf("failed to create validation request: %w", err) + } + + rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "components applied and generated") + + obj.Status.LastReconciledVersion = component.Version + status.MarkReady(r.EventRecorder, obj, "Applied version: %s", component.Version) + + return ctrl.Result{}, nil +} + +func (r *ProductDeploymentGeneratorReconciler) createSync(ctx context.Context, + obj *v1alpha1.ProductDeploymentGenerator, + prodDesc v1alpha1.ProductDescription, + component replicationv1.Component, + dir string, + cv ocm.ComponentVersionAccess, + project *projectv1.Project, +) (*gitv1alpha1.Sync, *cue.File, error) { + // Create top-level product folder + const perm = 0o777 + if err := os.Mkdir(filepath.Join(dir, obj.Name), perm); err != nil { status.MarkNotReady(r.EventRecorder, obj, v1alpha1.TemporaryFolderGenerationFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to create product folder: %w", err) + return nil, nil, fmt.Errorf("failed to create product folder: %w", err) } rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "generation kubernetes resources") productFolder := filepath.Join(dir, obj.Name) - productDeployment, values, err := r.createProductDeployment(ctx, obj, *prodDesc, component, productFolder, cv, project) + productDeployment, values, err := r.createProductDeployment(ctx, obj, prodDesc, component, productFolder, cv, project) if err != nil { - if errors.Is(err, unschedulableError) { + if errors.Is(err, errUnschedulable) { status.MarkNotReady(r.EventRecorder, obj, v1alpha1.ProductPipelineSchedulingFailedReason, err.Error()) // stop reconciling until fixed - return ctrl.Result{}, nil + return nil, nil, nil } status.MarkNotReady(r.EventRecorder, obj, v1alpha1.CreateProductPipelineFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to create product deployment: %w", err) + return nil, nil, fmt.Errorf("failed to create product deployment: %w", err) } // Note that we pass in the top level folder here. @@ -306,13 +362,13 @@ func (r *ProductDeploymentGeneratorReconciler) reconcile(ctx context.Context, ob if err != nil { status.MarkNotReady(r.EventRecorder, obj, v1alpha1.CreateSnapshotFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to create snapshot for product deployment: %w", err) + return nil, nil, fmt.Errorf("failed to create snapshot for product deployment: %w", err) } if project.Spec.Git.CommitTemplate == nil { status.MarkNotReady(r.EventRecorder, obj, v1alpha1.CommitTemplateEmptyReason, "commit template in project cannot be empty") - return ctrl.Result{}, fmt.Errorf("commit template in project cannot be empty") + return nil, nil, fmt.Errorf("commit template in project cannot be empty") } repositoryRef := project.Status.RepositoryRef.Name @@ -367,56 +423,10 @@ func (r *ProductDeploymentGeneratorReconciler) reconcile(ctx context.Context, ob }); err != nil { status.MarkNotReady(r.EventRecorder, obj, v1alpha1.CreateSyncFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to create sync request: %w", err) - } - - // Create the Validation Object. - validationSchema, err := values.Format() - if err != nil { - status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SchemaGenerationFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("failed to create sync request: %w", err) - } - - validation := &v1alpha1.Validation{ - ObjectMeta: metav1.ObjectMeta{ - Name: obj.Name + "-validation-" + hashedVersion, - Namespace: obj.Namespace, - Annotations: map[string]string{ - componentVersionAnnotationKey: component.Version, - componentNameAnnotationKey: component.Name, - }, - }, - Spec: v1alpha1.ValidationSpec{ - Schema: validationSchema, - ServiceAccountName: obj.Spec.ServiceAccountName, - Interval: metav1.Duration{Duration: 30 * time.Second}, - SyncRef: meta.NamespacedObjectReference{ - Name: sync.Name, - Namespace: sync.Namespace, - }, - }, - } - - if _, err := ctrl.CreateOrUpdate(ctx, r.Client, validation, func() error { - if validation.ObjectMeta.CreationTimestamp.IsZero() { - if err := controllerutil.SetOwnerReference(obj, validation, r.Scheme); err != nil { - return fmt.Errorf("failed to set owner to validation object: %w", err) - } - } - - return nil - }); err != nil { - status.MarkNotReady(r.EventRecorder, obj, v1alpha1.CreateValidationFailedReason, err.Error()) - - return ctrl.Result{}, fmt.Errorf("failed to create validation request: %w", err) + return nil, nil, fmt.Errorf("failed to create sync request: %w", err) } - rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "components applied and generated") - - obj.Status.LastReconciledVersion = component.Version - status.MarkReady(r.EventRecorder, obj, "Applied version: %s", component.Version) - - return ctrl.Result{}, nil + return sync, values, nil } func (r *ProductDeploymentGeneratorReconciler) createProductDeployment( @@ -498,7 +508,10 @@ func (r *ProductDeploymentGeneratorReconciler) createProductDeployment( values := schema var config *cue.File if existingConfigFile == nil { - config = values.EvalWithoutPrivateFields() + config, err = values.EvalWithoutPrivateFields() + if err != nil { + return nil, nil, err + } } else { config, err = existingConfigFile.Merge(schema) if err != nil { @@ -507,7 +520,10 @@ func (r *ProductDeploymentGeneratorReconciler) createProductDeployment( // Evaluate the configuration file to get the final values. // we do this here to make sure that the values file does not contain any // optional or expr fields that are not set. - config = config.Eval() + config, err = config.Eval() + if err != nil { + return nil, nil, fmt.Errorf("failed to evaluate config: %w", err) + } } err = config.Sanitize() @@ -525,11 +541,11 @@ func (r *ProductDeploymentGeneratorReconciler) createProductDeployment( return nil, nil, fmt.Errorf("failed to format values: %w", err) } - if err := os.WriteFile(filepath.Join(dir, "config.cue"), configBytes, 0o644); err != nil { + if err := os.WriteFile(filepath.Join(dir, "config.cue"), configBytes, v1alpha1.WritePermissions); err != nil { return nil, nil, fmt.Errorf("failed to write configuration values file: %w", err) } - if err := os.WriteFile(filepath.Join(dir, "README.md"), readme, 0o644); err != nil { + if err := os.WriteFile(filepath.Join(dir, "README.md"), readme, v1alpha1.WritePermissions); err != nil { return nil, nil, fmt.Errorf("failed to write readme file: %w", err) } @@ -557,19 +573,21 @@ func (r *ProductDeploymentGeneratorReconciler) createProductPipeline( var targetRole *v1alpha1.TargetRole if p.TargetRoleName == "" { - return v1alpha1.Pipeline{}, nil, fmt.Errorf("pipeline '%s' cannot be scheduled: %w", p.Name, unschedulableError) + return v1alpha1.Pipeline{}, nil, fmt.Errorf("pipeline '%s' cannot be scheduled: %w", p.Name, errUnschedulable) } // if the list is empty, select one from the available targets. for _, role := range description.Spec.TargetRoles { + role := role if role.Name == p.TargetRoleName { targetRole = &role.TargetRole + break } } if targetRole == nil { - return v1alpha1.Pipeline{}, nil, fmt.Errorf("failed to find a target role with name %s: %w", p.TargetRoleName, unschedulableError) + return v1alpha1.Pipeline{}, nil, fmt.Errorf("failed to find a target role with name %s: %w", p.TargetRoleName, errUnschedulable) } var schemaFile *cue.File @@ -599,7 +617,13 @@ func (r *ProductDeploymentGeneratorReconciler) createProductPipeline( }, schemaFile, nil } -func (r *ProductDeploymentGeneratorReconciler) createSnapshot(ctx context.Context, obj *v1alpha1.ProductDeploymentGenerator, productDeployment *v1alpha1.ProductDeployment, component replicationv1.Component, dir string) (string, error) { +func (r *ProductDeploymentGeneratorReconciler) createSnapshot( + ctx context.Context, + obj *v1alpha1.ProductDeploymentGenerator, + productDeployment *v1alpha1.ProductDeployment, + component replicationv1.Component, + dir string, +) (string, error) { serializer := json.NewSerializerWithOptions(json.DefaultMetaFactory, r.Scheme, r.Scheme, json.SerializerOptions{ Yaml: true, Pretty: true, @@ -613,7 +637,7 @@ func (r *ProductDeploymentGeneratorReconciler) createSnapshot(ctx context.Contex defer productDeploymentFile.Close() - if err := os.WriteFile(filepath.Join(dir, obj.Name, "kustomization.yaml"), []byte(kustomizationFile), 0o777); err != nil { + if err := os.WriteFile(filepath.Join(dir, obj.Name, "kustomization.yaml"), []byte(kustomizationFile), v1alpha1.WritePermissions); err != nil { return "", fmt.Errorf("failed to create kustomization file: %w", err) } @@ -662,7 +686,7 @@ func (r *ProductDeploymentGeneratorReconciler) increaseHeaders(instructions []by type headerTransformer struct{} -func (t *headerTransformer) Transform(doc *ast.Document, reader text.Reader, pctx parser.Context) { +func (t *headerTransformer) Transform(doc *ast.Document, _ text.Reader, _ parser.Context) { _ = ast.Walk(doc, func(node ast.Node, enter bool) (ast.WalkStatus, error) { if enter { heading, ok := node.(*ast.Heading) @@ -684,7 +708,7 @@ func (r *ProductDeploymentGeneratorReconciler) findGenerators(ctx context.Contex selectorTerm := client.ObjectKeyFromObject(o).String() generators := &v1alpha1.ProductDeploymentGeneratorList{} - if err := r.List(context.TODO(), generators, &client.ListOptions{ + if err := r.List(ctx, generators, &client.ListOptions{ FieldSelector: fields.OneTermEqualSelector(sourceKey, selectorTerm), }); err != nil { return []reconcile.Request{} @@ -706,12 +730,49 @@ func (r *ProductDeploymentGeneratorReconciler) findGenerators(ctx context.Contex // hashComponentVersion provides a small chunk of a hexadecimal format for a version. func (r *ProductDeploymentGeneratorReconciler) hashComponentVersion(version string) string { - h := sha1.New() + h := sha1.New() //nolint:gosec // good enough for our purposes h.Write([]byte(version)) return hex.EncodeToString(h.Sum(nil))[:8] } +func (r *ProductDeploymentGeneratorReconciler) shouldRun( + obj *v1alpha1.ProductDeploymentGenerator, + subscription *replicationv1.ComponentSubscription, +) (bool, error) { + if !conditions.IsReady(subscription) { + status.MarkNotReady(r.EventRecorder, obj, v1alpha1.ComponentSubscriptionNotReadyReason, "component subscription not ready yet") + + return false, nil + } + + if obj.Status.LastReconciledVersion == "" { + return true, nil + } + + lastReconciledGeneratorVersion, err := semver.NewVersion(obj.Status.LastReconciledVersion) + if err != nil { + status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SemverParseFailedReason, err.Error()) + + return false, fmt.Errorf("failed to parse last reconciled version: %w", err) + } + + lastReconciledSubscription, err := semver.NewVersion(subscription.Status.LastAppliedVersion) + if err != nil { + status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SemverParseFailedReason, err.Error()) + + return false, fmt.Errorf("failed to parse last reconciled version: %w", err) + } + + if lastReconciledSubscription.Equal(lastReconciledGeneratorVersion) || lastReconciledSubscription.LessThan(lastReconciledGeneratorVersion) { + status.MarkReady(r.EventRecorder, obj, "Reconciliation success") + + return false, nil + } + + return true, nil +} + // fetchExistingConfigFile gathers existing config.cue values. If it doesn't exist it returns an empty file and no error. func fetchExistingConfigFile(ctx context.Context, r client.Client, productName string, project *projectv1.Project) (*cue.File, error) { repoName, repoNamespace, err := FetchGitRepositoryFromProjectInventory(project) diff --git a/controllers/productdeploymentpipeline_controller.go b/controllers/productdeploymentpipeline_controller.go index 81b9ede..9ffab33 100644 --- a/controllers/productdeploymentpipeline_controller.go +++ b/controllers/productdeploymentpipeline_controller.go @@ -28,10 +28,14 @@ import ( mpasv1alpha1 "github.com/open-component-model/mpas-product-controller/api/v1alpha1" mpasocm "github.com/open-component-model/mpas-product-controller/pkg/ocm" - projectv1 "github.com/open-component-model/mpas-project-controller/api/v1alpha1" ocmv1alpha1 "github.com/open-component-model/ocm-controller/api/v1alpha1" ) +const ( + defaultPipelineRequeue = 10 * time.Minute + quickRequeue = 10 * time.Second +) + // ProductDeploymentPipelineReconciler reconciles a ProductDeploymentPipeline object. type ProductDeploymentPipelineReconciler struct { client.Client @@ -112,15 +116,8 @@ func (r *ProductDeploymentPipelineReconciler) Reconcile(ctx context.Context, req snapshotProvider = localization - project, err := GetProjectFromObjectNamespace(ctx, r.Client, owner, r.MpasSystemNamespace) - if err != nil { - status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.ProjectInNamespaceGetFailedReason, err.Error()) - - return ctrl.Result{}, fmt.Errorf("failed to find the project in the namespace: %w", err) - } - // Create Configuration - configuration, err := r.createOrUpdateConfiguration(ctx, obj, owner, localization, project) + configuration, err := r.createOrUpdateConfiguration(ctx, obj, localization) if err != nil { err := fmt.Errorf("failed to create configuration: %w", err) status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.CreateConfigurationFailedReason, err.Error()) @@ -142,7 +139,7 @@ func (r *ProductDeploymentPipelineReconciler) Reconcile(ctx context.Context, req if snapshotProvider.GetSnapshotName() == "" { logger.Info("snapshot hasn't been produced yet, requeuing pipeline", "pipeline", obj.Name) - return ctrl.Result{RequeueAfter: 10 * time.Second}, nil + return ctrl.Result{RequeueAfter: quickRequeue}, nil } obj.Status.SnapshotRef = &meta.NamespacedObjectReference{ @@ -158,9 +155,7 @@ func (r *ProductDeploymentPipelineReconciler) Reconcile(ctx context.Context, req func (r *ProductDeploymentPipelineReconciler) createOrUpdateConfiguration( ctx context.Context, obj *mpasv1alpha1.ProductDeploymentPipeline, - owner *mpasv1alpha1.ProductDeployment, localization *ocmv1alpha1.Localization, - project *projectv1.Project, ) (*ocmv1alpha1.Configuration, error) { if obj.Spec.Configuration.Rules.Name == "" { return nil, nil @@ -188,7 +183,7 @@ func (r *ProductDeploymentPipelineReconciler) createOrUpdateConfiguration( } spec := ocmv1alpha1.MutationSpec{ - Interval: metav1.Duration{Duration: 10 * time.Minute}, + Interval: metav1.Duration{Duration: defaultPipelineRequeue}, SourceRef: source, ConfigRef: &ocmv1alpha1.ObjectReference{ NamespacedObjectKindReference: meta.NamespacedObjectKindReference{ @@ -204,6 +199,7 @@ func (r *ProductDeploymentPipelineReconciler) createOrUpdateConfiguration( Name: obj.Spec.ConfigMapRef, }, Key: "values.yaml", + //nolint:godox // yep // TODO: This means that's its a must have in the config.cue file // Reflect this in the cue file SubPath: obj.Name, @@ -225,15 +221,18 @@ func (r *ProductDeploymentPipelineReconciler) createOrUpdateConfiguration( } } - if configuration.Spec.SourceRef.ResourceRef == nil || configuration.Spec.SourceRef.ResourceRef.Name != spec.SourceRef.ResourceRef.Name { + if configuration.Spec.SourceRef.ResourceRef == nil || + configuration.Spec.SourceRef.ResourceRef.Name != spec.SourceRef.ResourceRef.Name { configuration.Spec = spec } - if configuration.Spec.ConfigRef.ResourceRef == nil || configuration.Spec.ConfigRef.ResourceRef.Name != spec.ConfigRef.ResourceRef.Name { + if configuration.Spec.ConfigRef.ResourceRef == nil || + configuration.Spec.ConfigRef.ResourceRef.Name != spec.ConfigRef.ResourceRef.Name { configuration.Spec = spec } - if configuration.Spec.ValuesFrom.ConfigMapSource == nil || configuration.Spec.ValuesFrom.ConfigMapSource.SourceRef.Name != spec.ValuesFrom.ConfigMapSource.SourceRef.Name { + if configuration.Spec.ValuesFrom.ConfigMapSource == nil || + configuration.Spec.ValuesFrom.ConfigMapSource.SourceRef.Name != spec.ValuesFrom.ConfigMapSource.SourceRef.Name { configuration.Spec = spec } @@ -245,7 +244,10 @@ func (r *ProductDeploymentPipelineReconciler) createOrUpdateConfiguration( return configuration, nil } -func (r *ProductDeploymentPipelineReconciler) createOrUpdateLocalization(ctx context.Context, obj *mpasv1alpha1.ProductDeploymentPipeline) (*ocmv1alpha1.Localization, error) { +func (r *ProductDeploymentPipelineReconciler) createOrUpdateLocalization( + ctx context.Context, + obj *mpasv1alpha1.ProductDeploymentPipeline, +) (*ocmv1alpha1.Localization, error) { if obj.Spec.Localization.Name == "" { return nil, nil } @@ -259,7 +261,7 @@ func (r *ProductDeploymentPipelineReconciler) createOrUpdateLocalization(ctx con Namespace: obj.Namespace, }, Spec: ocmv1alpha1.MutationSpec{ - Interval: metav1.Duration{Duration: 10 * time.Minute}, + Interval: metav1.Duration{Duration: defaultPipelineRequeue}, SourceRef: ocmv1alpha1.ObjectReference{ NamespacedObjectKindReference: meta.NamespacedObjectKindReference{ Kind: "ComponentVersion", diff --git a/controllers/productdeploymentpipeline_scheduler.go b/controllers/productdeploymentpipeline_scheduler.go index 014eeff..bec981f 100644 --- a/controllers/productdeploymentpipeline_scheduler.go +++ b/controllers/productdeploymentpipeline_scheduler.go @@ -8,7 +8,6 @@ import ( "context" "errors" "fmt" - "time" eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" "github.com/fluxcd/pkg/apis/meta" @@ -69,7 +68,7 @@ func (r *ProductDeploymentPipelineScheduler) Reconcile(ctx context.Context, req if obj.Status.SnapshotRef == nil || obj.Status.SnapshotRef.Name == "" { logger.Info("snapshot has not yet been set up, requeuing...") - return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + return ctrl.Result{RequeueAfter: defaultValidationRequeue}, nil } snapshot := &v1alpha1.Snapshot{} @@ -77,7 +76,7 @@ func (r *ProductDeploymentPipelineScheduler) Reconcile(ctx context.Context, req if apierrors.IsNotFound(err) { logger.Info("snapshot not found yet, requeuing") - return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + return ctrl.Result{RequeueAfter: defaultValidationRequeue}, nil } return ctrl.Result{}, fmt.Errorf("failed to retrieve snapshot object: %w", err) @@ -86,7 +85,7 @@ func (r *ProductDeploymentPipelineScheduler) Reconcile(ctx context.Context, req if !conditions.IsTrue(snapshot, meta.ReadyCondition) { logger.Info("snapshot found but is not ready yet, requeuing...") - return ctrl.Result{RequeueAfter: 5 * time.Second}, nil + return ctrl.Result{RequeueAfter: defaultValidationRequeue}, nil } patchHelper := patch.NewSerialPatcher(obj, r.Client) diff --git a/controllers/select_target.go b/controllers/select_target.go index 874c360..a585f31 100644 --- a/controllers/select_target.go +++ b/controllers/select_target.go @@ -49,13 +49,18 @@ func (r *ProductDeploymentPipelineScheduler) SelectTarget(ctx context.Context, r case 1: return targets[0], nil default: - r := rand.New(rand.NewSource(time.Now().Unix())) + r := rand.New(rand.NewSource(time.Now().Unix())) //nolint:gosec // good enough index := r.Intn(len(targets)) + return targets[index], nil } } -func (r *ProductDeploymentPipelineScheduler) searchForTargetsInNamespace(ctx context.Context, selector v1.LabelSelector, namespace string) (*v1alpha1.TargetList, error) { +func (r *ProductDeploymentPipelineScheduler) searchForTargetsInNamespace( + ctx context.Context, + selector v1.LabelSelector, + namespace string, +) (*v1alpha1.TargetList, error) { targetList := &v1alpha1.TargetList{} m, err := v1.LabelSelectorAsSelector(&selector) if err != nil { @@ -64,6 +69,7 @@ func (r *ProductDeploymentPipelineScheduler) searchForTargetsInNamespace(ctx con // We can't use client.MatchingFields for spec.type since we don't have a controller for Target // and thus, we can't index the type field. + //nolint:godox // yep //TODO: post MVP add controller for targets if err := r.List(ctx, targetList, client.InNamespace(namespace), client.MatchingLabelsSelector{ Selector: m, diff --git a/controllers/target_controller.go b/controllers/target_controller.go index 55de2a7..027b078 100644 --- a/controllers/target_controller.go +++ b/controllers/target_controller.go @@ -54,7 +54,7 @@ func getPatchOptions(ownedConditions []string, controllerName string) []patch.Op //+kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;watch //+kubebuilder:rbac:groups="",resources=serviceaccounts;namespaces,verbs=get;list;watch;create;update;patch;delete -// TargetReconciler reconciles a Target object +// TargetReconciler reconciles a Target object. type TargetReconciler struct { client.Client Scheme *runtime.Scheme @@ -64,8 +64,9 @@ type TargetReconciler struct { } // SetupWithManager sets up the controller with the Manager. -func (r *TargetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error { +func (r *TargetReconciler) SetupWithManager(_ context.Context, mgr ctrl.Manager) error { r.patchOptions = getPatchOptions(targetOwnedConditions, r.ControllerName) + return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.Target{}, builder.WithPredicates( predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), @@ -79,7 +80,7 @@ func (r *TargetReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manage func (r *TargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, retErr error) { logger := log.FromContext(ctx) - logger.V(4).Info("starting reconciliation", "target", req.NamespacedName) + logger.V(v1alpha1.LevelDebug).Info("starting reconciliation", "target", req.NamespacedName) obj := &v1alpha1.Target{} if err := r.Get(ctx, req.NamespacedName, obj); err != nil { @@ -102,65 +103,25 @@ func (r *TargetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ c if err := serialPatcher.Patch(ctx, obj, patchOpts...); err != nil { // Ignore patch error "not found" when the object is being deleted. if !obj.GetDeletionTimestamp().IsZero() { - err = kerrors.FilterOut(err, func(e error) bool { return apierrors.IsNotFound(e) }) + err = kerrors.FilterOut(err, apierrors.IsNotFound) } retErr = kerrors.NewAggregate([]error{retErr, err}) } - }() return r.reconcile(ctx, serialPatcher, obj) } -func (r *TargetReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher, obj *v1alpha1.Target) (result ctrl.Result, retErr error) { +func (r *TargetReconciler) reconcile( + ctx context.Context, + sp *patch.SerialPatcher, + obj *v1alpha1.Target, +) (result ctrl.Result, retErr error) { oldObj := obj.DeepCopy() - defer func() { - // If it's stalled, ensure reconciling is removed. - if sc := conditions.Get(obj, meta.StalledCondition); sc != nil && sc.Status == metav1.ConditionTrue { - conditions.Delete(obj, meta.ReconcilingCondition) - } - - // Check if it's a successful reconciliation. - if result.RequeueAfter == obj.GetRequeueAfter() && !result.Requeue && retErr == nil { - conditions.Delete(obj, meta.ReconcilingCondition) - if ready := conditions.Get(obj, meta.ReadyCondition); ready != nil && - ready.Status == metav1.ConditionFalse && !conditions.IsStalled(obj) { - retErr = errors.New(conditions.GetMessage(obj, meta.ReadyCondition)) - } - } - - if conditions.IsReconciling(obj) { - reconciling := conditions.Get(obj, meta.ReconcilingCondition) - reconciling.Reason = meta.ProgressingWithRetryReason - conditions.Set(obj, reconciling) - } - - // If it's still a successful reconciliation and it's not reconciling or - // stalled, mark Ready=True. - if !conditions.IsReconciling(obj) && !conditions.IsStalled(obj) && - retErr == nil && result.RequeueAfter == obj.GetRequeueAfter() { - conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "Target is ready") - } - - // Emit events when object's state changes. - ready := conditions.Get(obj, meta.ReadyCondition) - // Became ready from not ready. - if !conditions.IsReady(oldObj) && conditions.IsReady(obj) { - r.eventLogf(ctx, obj, corev1.EventTypeNormal, ready.Reason, ready.Message) - } - // Became not ready from ready. - if conditions.IsReady(oldObj) && !conditions.IsReady(obj) { - r.eventLogf(ctx, obj, corev1.EventTypeWarning, ready.Reason, ready.Message) - } - - // Apply jitter. - if result.RequeueAfter == obj.GetRequeueAfter() { - result.RequeueAfter = jitter.JitteredIntervalDuration(result.RequeueAfter) - } - }() + defer r.setStatus(ctx, obj, oldObj, retErr, &result) - // delete reconciling and stalled conditions if they exis + // delete reconciling and stalled conditions if they exist rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress") if obj.Generation != obj.Status.ObservedGeneration { @@ -175,6 +136,7 @@ func (r *TargetReconciler) reconcile(ctx context.Context, sp *patch.SerialPatche if err := yaml.Unmarshal(obj.Spec.Access.Raw, &kubernetesAccess); err != nil { conditions.MarkStalled(obj, v1alpha1.AccessInvalidReason, err.Error()) conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.AccessInvalidReason, err.Error()) + return ctrl.Result{}, err } @@ -189,9 +151,9 @@ func (r *TargetReconciler) reconcile(ctx context.Context, sp *patch.SerialPatche _, err := controllerutil.CreateOrUpdate(ctx, r.Client, ns, func() error { return nil }) - if err != nil { conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.NamespaceCreateOrUpdateFailedReason, err.Error()) + return ctrl.Result{}, fmt.Errorf("error reconciling namespace: %w", err) } @@ -201,57 +163,114 @@ func (r *TargetReconciler) reconcile(ctx context.Context, sp *patch.SerialPatche // no longer specified in the Target spec, because it may be used by // other resources. if obj.Spec.ServiceAccountName != "" { - // get secrets with the target label if it exists - secrets := &corev1.SecretList{} - if obj.Spec.SecretsSelector != nil { - if err = r.List(ctx, secrets, client.MatchingLabels(obj.Spec.SecretsSelector.MatchLabels), client.InNamespace(ns.Name)); err != nil { - conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.SecretRetrievalFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error retrieving secrets: %w", err) - } + if err := r.reconcileServiceAccount(ctx, obj, ns, kubernetesAccess); err != nil { + conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.ServiceAccountCreateOrUpdateFailedReason, err.Error()) - if len(secrets.Items) == 0 { - conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.SecretRetrievalFailedReason, "no secrets found") - return ctrl.Result{}, fmt.Errorf("no secrets found") - } + return ctrl.Result{}, fmt.Errorf("error reconciling service account: %w", err) + } + } + + // Remove any stale Ready condition, most likely False, set above. Its value + // is derived from the overall result of the reconciliation in the deferred + // block at the very end. + conditions.Delete(obj, meta.ReadyCondition) + + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil +} + +func (r *TargetReconciler) setStatus(ctx context.Context, obj, oldObj *v1alpha1.Target, retErr error, result *ctrl.Result) { + // If it's stalled, ensure reconciling is removed. + if sc := conditions.Get(obj, meta.StalledCondition); sc != nil && sc.Status == metav1.ConditionTrue { + conditions.Delete(obj, meta.ReconcilingCondition) + } + + // Check if it's a successful reconciliation. + if result.RequeueAfter == obj.GetRequeueAfter() && !result.Requeue && retErr == nil { + conditions.Delete(obj, meta.ReconcilingCondition) + if ready := conditions.Get(obj, meta.ReadyCondition); ready != nil && + ready.Status == metav1.ConditionFalse && !conditions.IsStalled(obj) { + retErr = errors.New(conditions.GetMessage(obj, meta.ReadyCondition)) } + } - sa := &corev1.ServiceAccount{ - ObjectMeta: metav1.ObjectMeta{ - Name: obj.Spec.ServiceAccountName, - Namespace: kubernetesAccess.TargetNamespace, - }, + if conditions.IsReconciling(obj) { + reconciling := conditions.Get(obj, meta.ReconcilingCondition) + reconciling.Reason = meta.ProgressingWithRetryReason + conditions.Set(obj, reconciling) + } + + // If it's still a successful reconciliation, and it's not reconciling or + // stalled, mark Ready=True. + if !conditions.IsReconciling(obj) && !conditions.IsStalled(obj) && + retErr == nil && result.RequeueAfter == obj.GetRequeueAfter() { + conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "Target is ready") + } + + // Emit events when object's state changes. + ready := conditions.Get(obj, meta.ReadyCondition) + // Became ready from not ready. + if !conditions.IsReady(oldObj) && conditions.IsReady(obj) { + r.eventLogf(ctx, obj, corev1.EventTypeNormal, ready.Reason, ready.Message) + } + // Became not ready from ready. + if conditions.IsReady(oldObj) && !conditions.IsReady(obj) { + r.eventLogf(ctx, obj, corev1.EventTypeWarning, ready.Reason, ready.Message) + } + + // Apply jitter. + if result.RequeueAfter == obj.GetRequeueAfter() { + result.RequeueAfter = jitter.JitteredIntervalDuration(result.RequeueAfter) + } +} + +func (r *TargetReconciler) reconcileServiceAccount( + ctx context.Context, + obj *v1alpha1.Target, + ns *corev1.Namespace, + kubernetesAccess v1alpha1.KubernetesAccess, +) error { + // get secrets with the target label if it exists + secrets := &corev1.SecretList{} + if obj.Spec.SecretsSelector != nil { + if err := r.List(ctx, secrets, client.MatchingLabels(obj.Spec.SecretsSelector.MatchLabels), client.InNamespace(ns.Name)); err != nil { + return fmt.Errorf("error retrieving secrets: %w", err) } - // if the secrets exist, add it to the service account - var imagePullSecrets []corev1.LocalObjectReference - for _, secret := range secrets.Items { - imagePullSecrets = append(imagePullSecrets, corev1.LocalObjectReference{ - Name: secret.Name, - }) + if len(secrets.Items) == 0 { + return fmt.Errorf("no secrets found") } + } - _, err = controllerutil.CreateOrUpdate(ctx, r.Client, sa, func() error { - if !equalImagePullSecretSlices(sa.ImagePullSecrets, imagePullSecrets) { - sa.ImagePullSecrets = imagePullSecrets - } - return nil + sa := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: obj.Spec.ServiceAccountName, + Namespace: kubernetesAccess.TargetNamespace, + }, + } + + // if the secrets exist, add it to the service account + var imagePullSecrets []corev1.LocalObjectReference + for _, secret := range secrets.Items { + imagePullSecrets = append(imagePullSecrets, corev1.LocalObjectReference{ + Name: secret.Name, }) + } - if err != nil { - conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.ServiceAccountCreateOrUpdateFailedReason, err.Error()) - return ctrl.Result{}, fmt.Errorf("error reconciling service account: %w", err) + _, err := controllerutil.CreateOrUpdate(ctx, r.Client, sa, func() error { + if !equalImagePullSecretSlices(sa.ImagePullSecrets, imagePullSecrets) { + sa.ImagePullSecrets = imagePullSecrets } - } - // Remove any stale Ready condition, most likely False, set above. Its value - // is derived from the overall result of the reconciliation in the deferred - // block at the very end. - conditions.Delete(obj, meta.ReadyCondition) + return nil + }) + if err != nil { + return fmt.Errorf("failed to reconcile service account: %w", err) + } - return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil + return nil } -func (r *TargetReconciler) eventLogf(ctx context.Context, obj runtime.Object, eventType string, reason string, messageFmt string, args ...interface{}) { +func (r *TargetReconciler) eventLogf(ctx context.Context, obj runtime.Object, eventType, reason, messageFmt string, args ...interface{}) { msg := fmt.Sprintf(messageFmt, args...) // Log and emit event. if eventType == corev1.EventTypeWarning { @@ -271,5 +290,6 @@ func equalImagePullSecretSlices(a, b []corev1.LocalObjectReference) bool { return false } } + return true } diff --git a/controllers/validation_controller.go b/controllers/validation_controller.go index bd17bea..0ee47f6 100644 --- a/controllers/validation_controller.go +++ b/controllers/validation_controller.go @@ -36,7 +36,9 @@ import ( "github.com/open-component-model/mpas-product-controller/pkg/validators" ) -// ValidationReconciler reconciles a Validation object +const defaultValidationRequeue = 5 * time.Second + +// ValidationReconciler reconciles a Validation object. type ValidationReconciler struct { client.Client Scheme *runtime.Scheme @@ -48,19 +50,28 @@ type ValidationReconciler struct { // SetupWithManager sets up the controller with the Manager. func (r *ValidationReconciler) SetupWithManager(mgr ctrl.Manager) error { - if err := mgr.GetFieldIndexer().IndexField(context.Background(), &sourcebeta2.GitRepository{}, controllerMetadataKey, func(rawObj client.Object) []string { - repository := rawObj.(*sourcebeta2.GitRepository) - owner := metav1.GetControllerOf(repository) - if owner == nil { - return nil - } + if err := mgr.GetFieldIndexer().IndexField( + context.Background(), + &sourcebeta2.GitRepository{}, + controllerMetadataKey, + func(rawObj client.Object) []string { + repository, ok := rawObj.(*sourcebeta2.GitRepository) + if !ok { + return nil + } - if owner.APIVersion != mpasv1alpha1.GroupVersion.String() || owner.Kind != "Validation" { - return nil - } + owner := metav1.GetControllerOf(repository) + if owner == nil { + return nil + } - return []string{owner.Name} - }); err != nil { + if owner.APIVersion != mpasv1alpha1.GroupVersion.String() || owner.Kind != "Validation" { + return nil + } + + return []string{owner.Name} + }, + ); err != nil { return err } @@ -90,7 +101,7 @@ func (r *ValidationReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, fmt.Errorf("failed to retrieve validation object: %w", err) } - logger.V(4).Info("reconciling the validation", "status", obj.Status) + logger.V(mpasv1alpha1.LevelDebug).Info("reconciling the validation", "status", obj.Status) objPatcher := patch.NewSerialPatcher(obj, r.Client) @@ -116,93 +127,19 @@ func (r *ValidationReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, fmt.Errorf("expected one owner, got: %d", len(owners)) } - sync := &gitv1alpha1.Sync{} - if err := r.Get(ctx, types.NamespacedName{Name: obj.Spec.SyncRef.Name, Namespace: obj.Spec.SyncRef.Namespace}, sync); err != nil { - if apierrors.IsNotFound(err) { - return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil - } - - return ctrl.Result{}, fmt.Errorf("failed to get owner object: %w", err) - } + // These will be outputs of the below should run method. + var ( + sync = &gitv1alpha1.Sync{} + repository = &gitmpasv1alpha1.Repository{} + gitRepository = &sourcebeta2.GitRepository{} + ) - if !conditions.IsReady(sync) || sync.Status.PullRequestID == 0 { - logger.Info("sync request isn't done yet... waiting") - - return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil - } - - project, err := GetProjectFromObjectNamespace(ctx, r.Client, sync, r.MpasSystemNamespace) + run, result, err := r.shouldRun(ctx, owners, obj, sync, repository, gitRepository) if err != nil { - conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.ProjectInNamespaceGetFailedReason, err.Error()) - status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.ProjectInNamespaceGetFailedReason, err.Error()) - - return ctrl.Result{}, fmt.Errorf("failed to find the project in the namespace: %w", err) - } - - if !conditions.IsReady(project) { - logger.Info("project not ready yet") - - return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil - } - - if project.Status.RepositoryRef == nil { - logger.Info("no repository information is provided yet") - - return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil - } - - repository := &gitmpasv1alpha1.Repository{} - if err := r.Get(ctx, types.NamespacedName{ - Name: project.Status.RepositoryRef.Name, - Namespace: project.Status.RepositoryRef.Namespace, - }, repository); err != nil { return ctrl.Result{}, err } - - if conditions.IsTrue(obj, meta.ReadyCondition) { - merged, err := r.Validator.IsMergedOrClosed(ctx, *repository, *sync) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to fetch pull request status: %w", err) - } - - if merged { - logger.Info("validation pull request is merged/closed, removing git repository") - if err := r.deleteGitRepository(ctx, obj.Status.GitRepositoryRef); err != nil { - status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.GitRepositoryCleanUpFailedReason, err.Error()) - - return ctrl.Result{}, fmt.Errorf("failed to delete GitRepository tracking the values file: %w", err) - } - - // Stop reconciling this validation any further. - return ctrl.Result{}, nil - } - } - - if obj.Status.GitRepositoryRef == nil { - logger.Info("creating git repository to track value changes") - // create gitrepository to track values file and immediately requeue - ref, err := r.createValueFileGitRepository(ctx, obj, owners[0].Name, sync.Status.PullRequestID, *repository) - - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to create value tracking git repo: %w", err) - } - - obj.Status.GitRepositoryRef = &ref - - // This will requeue anyway, since we are watching GitRepository objects. One the GitRepository is IsReady - // it should requeue a run for this validation. - return ctrl.Result{}, nil - } - - gitRepository := &sourcebeta2.GitRepository{} - if err := r.Get(ctx, types.NamespacedName{Name: obj.Status.GitRepositoryRef.Name, Namespace: obj.Status.GitRepositoryRef.Namespace}, gitRepository); err != nil { - if apierrors.IsNotFound(err) { - logger.Info("values file tracking gitrepository already removed") - - return ctrl.Result{}, nil - } - - return ctrl.Result{}, fmt.Errorf("failed to find values file tracking git repository: %w", err) + if !run { + return result, nil } artifact := gitRepository.GetArtifact() @@ -228,23 +165,27 @@ func (r *ValidationReconciler) Reconcile(ctx context.Context, req ctrl.Request) config, err := cue.New("config", "", data) if err != nil { status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.ValidationFailedReason, err.Error()) + return ctrl.Result{}, fmt.Errorf("failed to generate schema: %w", err) } schema, err := cue.New("schema", "", obj.Spec.Schema) if err != nil { status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.ValidationFailedReason, err.Error()) + return ctrl.Result{}, fmt.Errorf("failed to generate schema: %w", err) } err = config.Validate(schema) if err != nil { status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.ValidationFailedReason, err.Error()) + return ctrl.Result{}, fmt.Errorf("failed to validate config: %w", err) } if err := r.Validator.PassValidation(ctx, *repository, *sync); err != nil { status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.ValidationFailedReason, err.Error()) + return ctrl.Result{}, fmt.Errorf("failed to set pull request checks to success: %w", err) } @@ -256,7 +197,13 @@ func (r *ValidationReconciler) Reconcile(ctx context.Context, req ctrl.Request) } // createValueFileGitRepository creates a GitRepository that tracks changes on a branch. -func (r *ValidationReconciler) createValueFileGitRepository(ctx context.Context, obj *mpasv1alpha1.Validation, productName string, pullId int, repository gitmpasv1alpha1.Repository) (meta.NamespacedObjectReference, error) { +func (r *ValidationReconciler) createValueFileGitRepository( + ctx context.Context, + obj *mpasv1alpha1.Validation, + productName string, + pullID int, + repository gitmpasv1alpha1.Repository, +) (meta.NamespacedObjectReference, error) { repo := &sourcebeta2.GitRepository{ ObjectMeta: metav1.ObjectMeta{ Name: obj.Name + "-values-repo", @@ -267,9 +214,9 @@ func (r *ValidationReconciler) createValueFileGitRepository(ctx context.Context, SecretRef: &meta.LocalObjectReference{ Name: repository.Spec.Credentials.SecretRef.Name, }, - Interval: metav1.Duration{Duration: 5 * time.Second}, + Interval: metav1.Duration{Duration: defaultValidationRequeue}, Reference: &sourcebeta2.GitRepositoryRef{ - Name: fmt.Sprintf("refs/pull/%d/head", pullId), + Name: fmt.Sprintf("refs/pull/%d/head", pullID), }, Ignore: pointer.String(fmt.Sprintf(`# exclude all /* @@ -312,3 +259,104 @@ func (r *ValidationReconciler) deleteGitRepository(ctx context.Context, ref *met return r.Delete(ctx, repo) } + +func (r *ValidationReconciler) shouldRun( + ctx context.Context, + owners []metav1.OwnerReference, + obj *mpasv1alpha1.Validation, + sync *gitv1alpha1.Sync, + repository *gitmpasv1alpha1.Repository, + gitRepository *sourcebeta2.GitRepository, +) (bool, ctrl.Result, error) { + logger := log.FromContext(ctx) + + if err := r.Get(ctx, types.NamespacedName{Name: obj.Spec.SyncRef.Name, Namespace: obj.Spec.SyncRef.Namespace}, sync); err != nil { + if apierrors.IsNotFound(err) { + return false, ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil + } + + return false, ctrl.Result{}, fmt.Errorf("failed to get owner object: %w", err) + } + + if !conditions.IsReady(sync) || sync.Status.PullRequestID == 0 { + logger.Info("sync request isn't done yet... waiting") + + return false, ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil + } + + project, err := GetProjectFromObjectNamespace(ctx, r.Client, sync, r.MpasSystemNamespace) + if err != nil { + conditions.MarkFalse(obj, meta.ReadyCondition, mpasv1alpha1.ProjectInNamespaceGetFailedReason, err.Error()) + status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.ProjectInNamespaceGetFailedReason, err.Error()) + + return false, ctrl.Result{}, fmt.Errorf("failed to find the project in the namespace: %w", err) + } + + if !conditions.IsReady(project) { + logger.Info("project not ready yet") + + return false, ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil + } + + if project.Status.RepositoryRef == nil { + logger.Info("no repository information is provided yet") + + return false, ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil + } + + if err := r.Get(ctx, types.NamespacedName{ + Name: project.Status.RepositoryRef.Name, + Namespace: project.Status.RepositoryRef.Namespace, + }, repository); err != nil { + return false, ctrl.Result{}, err + } + + if conditions.IsTrue(obj, meta.ReadyCondition) { + merged, err := r.Validator.IsMergedOrClosed(ctx, *repository, *sync) + if err != nil { + return false, ctrl.Result{}, fmt.Errorf("failed to fetch pull request status: %w", err) + } + + if merged { + logger.Info("validation pull request is merged/closed, removing git repository") + if err := r.deleteGitRepository(ctx, obj.Status.GitRepositoryRef); err != nil { + status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.GitRepositoryCleanUpFailedReason, err.Error()) + + return false, ctrl.Result{}, fmt.Errorf("failed to delete GitRepository tracking the values file: %w", err) + } + + // Stop reconciling this validation any further. + return false, ctrl.Result{}, nil + } + } + + if obj.Status.GitRepositoryRef == nil { + logger.Info("creating git repository to track value changes") + // create gitrepository to track values file and immediately requeue + ref, err := r.createValueFileGitRepository(ctx, obj, owners[0].Name, sync.Status.PullRequestID, *repository) + if err != nil { + return false, ctrl.Result{}, fmt.Errorf("failed to create value tracking git repo: %w", err) + } + + obj.Status.GitRepositoryRef = &ref + + // This will requeue anyway, since we are watching GitRepository objects. One the GitRepository is IsReady + // it should requeue a run for this validation. + return false, ctrl.Result{}, nil + } + + if err := r.Get(ctx, types.NamespacedName{ + Name: obj.Status.GitRepositoryRef.Name, + Namespace: obj.Status.GitRepositoryRef.Namespace, + }, gitRepository); err != nil { + if apierrors.IsNotFound(err) { + logger.Info("values file tracking gitrepository already removed") + + return false, ctrl.Result{}, nil + } + + return false, ctrl.Result{}, fmt.Errorf("failed to find values file tracking git repository: %w", err) + } + + return true, ctrl.Result{}, nil +} diff --git a/internal/cue/cue.go b/internal/cue/cue.go index acead8c..bd2f561 100644 --- a/internal/cue/cue.go +++ b/internal/cue/cue.go @@ -47,6 +47,7 @@ func New(name, filepath string, src any) (*File, error) { if err != nil { return nil, fmt.Errorf(fmt.Errorf("failed to parse cue file: %w", err).Error()) } + return &File{ Name: name, ctx: ctx, @@ -66,6 +67,7 @@ func (f *File) SchemaVersion() (string, error) { if err != nil { return "", err } + return f.schemaVersion, nil } @@ -76,6 +78,7 @@ func (f *File) Comments() string { for _, s := range f.file.Comments() { comments += s.Text() } + return comments } @@ -89,6 +92,7 @@ func (f *File) value() cue.Value { } v := f.ctx.BuildFile(f.file) f.v = v + return f.v } @@ -99,7 +103,7 @@ func (f *File) deltaFrom(file *ast.File) cue.Value { // EvalWithoutPrivateFields evaluates the cue file and removes any private fields. // note: calling Eval() after Eval() EvalWithoutPrivateFields() will add the private fields back // while removing the attributes. -func (f *File) EvalWithoutPrivateFields() *File { +func (f *File) EvalWithoutPrivateFields() (*File, error) { ctx := cuecontext.New() syn := []cue.Option{ cue.Final(), @@ -109,25 +113,16 @@ func (f *File) EvalWithoutPrivateFields() *File { cue.ErrorsAsValues(false), } - f = eval(f, syn) + var err error + f, err = eval(f, syn) + if err != nil { + return nil, err + } removePrivateFields(&f.file.Decls) f.v = ctx.BuildFile(f.file) f.setComments(f.v.Doc()) - return f -} -func containsPrivateFields(values []ast.Decl) bool { - for _, decl := range values { - if f, ok := decl.(*ast.Field); ok { - if len(f.Attrs) > 0 && f.Attrs[0].Text == "@private(true)" { - return true - } - if val, ok := f.Value.(*ast.StructLit); ok { - return containsPrivateFields(val.Elts) - } - } - } - return false + return f, nil } func removePrivateFields(values *[]ast.Decl) { @@ -176,7 +171,7 @@ func (f *File) Merge(schema *File, parents ...cue.Selector) (*File, error) { return nil, err } - return Export(f.Name, v), nil + return Export(f.Name, v) } // Sanitize makes sure the cue file is well formed. @@ -190,6 +185,7 @@ func (f *File) Validate(schema *File) error { if err != nil { return err } + return unified.Vet() } @@ -200,12 +196,15 @@ func (f *File) Unify(files []*File) (*File, error) { for _, file := range files { v = unify(v, file.value()) } - opts := append(defaultOpts, cue.Concrete(false)) + opts := defaultOpts + opts = append(opts, cue.Concrete(false)) + err := v.Validate(opts...) if err != nil { return nil, err } - return Export(f.Name, v), nil + + return Export(f.Name, v) } func unify(v1, v2 cue.Value) cue.Value { @@ -229,13 +228,14 @@ func (f *File) Yaml() ([]byte, error) { if err != nil { return nil, err } + return buf.Bytes(), nil } // Eval evaluates the cue file. // It expects the cue file to be well formed. // Optional fields and attributes are removed. -func (f *File) Eval() *File { +func (f *File) Eval() (*File, error) { syn := []cue.Option{ cue.Final(), cue.Definitions(true), @@ -249,19 +249,23 @@ func (f *File) Eval() *File { // eval evaluates the cue file. // It modifies the cue file in place. -func eval(f *File, syn []cue.Option) *File { +func eval(f *File, syn []cue.Option) (*File, error) { v := f.value() node := v.Syntax(syn...) - newfile := &ast.File{ + lit, ok := node.(*ast.StructLit) + if !ok { + return nil, fmt.Errorf("node is of unknown type %T", lit) + } + newFile := &ast.File{ Filename: f.file.Filename, - Decls: node.(*ast.StructLit).Elts, + Decls: lit.Elts, } - f.file = newfile - f.v = f.ctx.BuildFile(newfile) + f.file = newFile + f.v = f.ctx.BuildFile(newFile) f.setComments(v.Doc()) - return f + return f, nil } // Vet validates the cue file. @@ -283,32 +287,40 @@ func (f *File) Vet() error { return fmt.Errorf("failed: %w", err) } } + return nil } // Export exports the cue value to a cue file. -func Export(name string, v cue.Value, opts ...cue.Option) *File { +func Export(name string, v cue.Value, opts ...cue.Option) (*File, error) { opts = append(defaultOpts, opts...) ctx := cuecontext.New() + lit, ok := v.Syntax(opts...).(*ast.StructLit) + if !ok { + return nil, fmt.Errorf("syntax was of unknown type %T", lit) + } + file := &File{ Name: name, ctx: ctx, file: &ast.File{ - Decls: v.Syntax(opts...).(*ast.StructLit).Elts, + Decls: lit.Elts, }, v: v, } file.setComments(v.Doc()) - return file + + return file, nil } // src must be a string, []byte, or io.Reader. // if src is nil, the file is read from filepath. -func parse(ctx *cue.Context, filepath string, src any) (*ast.File, error) { +func parse(_ *cue.Context, filepath string, src any) (*ast.File, error) { tree, err := parser.ParseFile(filepath, src, parser.ParseComments) if err != nil { return nil, err } + return tree, nil } @@ -320,15 +332,16 @@ func fieldsDelta(schema, data cue.Value, opts []cue.Option, parents ...cue.Selec } for iter.Next() { - sel := append(parents, iter.Selector()) + sel := parents + sel = append(sel, iter.Selector()) // we need the absolute path to the field for lookup // but only the relative path to the field for m (missing fields) // as it will be appended to a list containing parent selectors absPath := cue.MakePath(sel...) relPath := cue.MakePath(append([]cue.Selector{}, iter.Selector())...) - //skip private fields based on the private attribute - //e.g. @private(true) + // skip private fields based on the private attribute + // e.g. @private(true). attr := iter.Value().Attribute(privateAttr) if err := attr.Err(); err == nil { continue @@ -339,7 +352,7 @@ func fieldsDelta(schema, data cue.Value, opts []cue.Option, parents ...cue.Selec m = append(m, relPath) } - switch iter.Value().Syntax().(type) { + switch iter.Value().Syntax().(type) { //nolint:gocritic // I like it this way. case *ast.StructLit: // recurse into the struct // to find missing fields @@ -351,7 +364,8 @@ func fieldsDelta(schema, data cue.Value, opts []cue.Option, parents ...cue.Selec // restore the selector prefix to the path for _, nv := range n { - nsel := append(sel[:], nv.Selectors()...) + nsel := sel + nsel = append(nsel, nv.Selectors()...) m = append(m, cue.MakePath(nsel...)) } } @@ -370,6 +384,7 @@ func generateDefaults(input cue.Value, fields []cue.Path, schemaVersion string) for _, p := range fields { paths = append(paths, p.String()) } + return makeValues(f, paths, schemaVersion) } @@ -378,7 +393,8 @@ func makeValues(iter *cue.Iterator, paths []string, schemaVersion string, parent for iter.Next() { var v ast.Expr value := iter.Value() - sel := append(parents, iter.Selector()) + sel := parents + sel = append(sel, iter.Selector()) path := cue.MakePath(sel...) if !slices.Contains(paths, path.String()) { @@ -388,7 +404,7 @@ func makeValues(iter *cue.Iterator, paths []string, schemaVersion string, parent field, hasDefaultValue := value.Default() if !hasDefaultValue && value.IsConcrete() { - switch value.Syntax(cue.Raw()).(type) { + switch t := value.Syntax(cue.Raw()).(type) { case *ast.StructLit: var rx []ast.Decl f, err := value.Fields(defaultOpts...) @@ -402,11 +418,17 @@ func makeValues(iter *cue.Iterator, paths []string, schemaVersion string, parent v = &ast.StructLit{ Elts: rx, } + case ast.Expr: + v = t default: - v = field.Syntax(cue.Raw()).(ast.Expr) + return nil, fmt.Errorf("unknown type: %T", t) } } else { - v = field.Syntax(cue.Raw()).(ast.Expr) + t, ok := field.Syntax(cue.Raw()).(ast.Expr) + if !ok { + return nil, fmt.Errorf("unknown type: %T", field.Syntax(cue.Raw())) + } + v = t } label, _ := value.Label() @@ -439,5 +461,6 @@ func makeValues(iter *cue.Iterator, paths []string, schemaVersion string, parent ast.SetComments(f, value.Doc()) result = append(result, f) } + return result, nil } diff --git a/internal/cue/cue_test.go b/internal/cue/cue_test.go index fba3478..e8aa513 100644 --- a/internal/cue/cue_test.go +++ b/internal/cue/cue_test.go @@ -7,6 +7,7 @@ package cue import ( "testing" + "cuelang.org/go/cue/ast" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -131,8 +132,10 @@ redis_url!: string & =~ "^https://" for _, tc := range testsCases { t.Run(tc.name, func(t *testing.T) { config := generateConfigFile(t, tc.config) - config = config.Eval() - err := config.Vet() + var err error + config, err = config.Eval() + require.NoError(t, err) + err = config.Vet() if tc.wantedErr != nil { err = tc.wantedErr(err) require.NoError(t, err) @@ -212,7 +215,8 @@ deployment: { config := generateConfigFile(t, tc.config) ok := containsPrivateFields(config.file.Decls) assert.True(t, ok) - f2 := config.EvalWithoutPrivateFields() + f2, err := config.EvalWithoutPrivateFields() + require.NoError(t, err) version, err := f2.SchemaVersion() require.NoError(t, err) @@ -225,6 +229,20 @@ deployment: { } } +func containsPrivateFields(values []ast.Decl) bool { + for _, decl := range values { + if f, ok := decl.(*ast.Field); ok { + if len(f.Attrs) > 0 && f.Attrs[0].Text == "@private(true)" { + return true + } + if val, ok := f.Value.(*ast.StructLit); ok { + return containsPrivateFields(val.Elts) + } + } + } + return false +} + func TestCue_Merge(t *testing.T) { schema := generateConfigFile(t, defaultSchema) modifiedSchema := generateConfigFile(t, modifiedSchema) diff --git a/main.go b/main.go index eb4f72b..bba6a8c 100644 --- a/main.go +++ b/main.go @@ -16,11 +16,6 @@ import ( "github.com/open-component-model/ocm-controller/pkg/snapshot" replicationv1 "github.com/open-component-model/replication-controller/api/v1alpha1" - "github.com/open-component-model/mpas-product-controller/pkg/deployers/kubernetes" - "github.com/open-component-model/mpas-product-controller/pkg/ocm" - "github.com/open-component-model/mpas-product-controller/pkg/validators/gitea" - "github.com/open-component-model/mpas-product-controller/pkg/validators/github" - // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. _ "k8s.io/client-go/plugin/pkg/client/auth" @@ -36,9 +31,14 @@ import ( mpasprojv1alpha1 "github.com/open-component-model/mpas-project-controller/api/v1alpha1" "github.com/fluxcd/pkg/runtime/events" + //+kubebuilder:scaffold:imports + mpasv1alpha1 "github.com/open-component-model/mpas-product-controller/api/v1alpha1" "github.com/open-component-model/mpas-product-controller/controllers" - //+kubebuilder:scaffold:imports + "github.com/open-component-model/mpas-product-controller/pkg/deployers/kubernetes" + "github.com/open-component-model/mpas-product-controller/pkg/ocm" + "github.com/open-component-model/mpas-product-controller/pkg/validators/gitea" + "github.com/open-component-model/mpas-product-controller/pkg/validators/github" ) const ( @@ -82,7 +82,12 @@ func main() { "Enable leader election for controller manager. "+ "Enabling this will ensure there is only one active controller manager.") flag.StringVar(&ociRegistryAddr, "oci-registry-addr", ":5000", "The address of the OCI registry.") - flag.StringVar(&mpasSystemNamespace, "mpas-system-namespace", defaultMpasSystemNamespace, "The namespace in which this controller is running in. This namespace is used to locate Project objects.") + flag.StringVar( + &mpasSystemNamespace, + "mpas-system-namespace", + defaultMpasSystemNamespace, + "The namespace in which this controller is running in. This namespace is used to locate Project objects.", + ) opts := zap.Options{ Development: true, @@ -92,10 +97,11 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + const metricsPort = 9443 mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, - Port: 9443, + Port: metricsPort, HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "b3469b71.ocm.software", @@ -203,5 +209,6 @@ func mustSetupEventRecorder(mgr ctrl.Manager, eventsAddr, controllerName string) setupLog.Error(err, "unable to create event recorder") os.Exit(1) } + return eventRecorder } diff --git a/pkg/ocm/ocm.go b/pkg/ocm/ocm.go index fe37527..54d3647 100644 --- a/pkg/ocm/ocm.go +++ b/pkg/ocm/ocm.go @@ -68,7 +68,7 @@ func (c *Client) CreateAuthenticatedOCMContext(ctx context.Context, serviceAccou func (c *Client) configureServiceAccountAccess(ctx context.Context, octx ocm.Context, serviceAccountName, namespace string) error { logger := log.FromContext(ctx) - logger.V(4).Info("configuring service account credentials") + logger.V(v1alpha1.LevelDebug).Info("configuring service account credentials") account := &corev1.ServiceAccount{} if err := c.client.Get(ctx, types.NamespacedName{ Name: serviceAccountName, @@ -77,7 +77,7 @@ func (c *Client) configureServiceAccountAccess(ctx context.Context, octx ocm.Con return fmt.Errorf("failed to fetch service account: %w", err) } - logger.V(4).Info("got service account", "name", account.GetName()) + logger.V(v1alpha1.LevelDebug).Info("got service account", "name", account.GetName()) for _, imagePullSecret := range account.ImagePullSecrets { secret := &corev1.Secret{} @@ -105,7 +105,7 @@ func (c *Client) configureServiceAccountAccess(ctx context.Context, octx ocm.Con } // GetComponentVersion returns a component Version. It's the caller's responsibility to clean it up and close the component Version once done with it. -func (c *Client) GetComponentVersion(ctx context.Context, octx ocm.Context, url, name, version string) (ocm.ComponentVersionAccess, error) { +func (c *Client) GetComponentVersion(_ context.Context, octx ocm.Context, url, name, version string) (ocm.ComponentVersionAccess, error) { repoSpec := ocireg.NewRepositorySpec(url, nil) repo, err := octx.RepositoryForSpec(repoSpec) if err != nil { @@ -121,11 +121,10 @@ func (c *Client) GetComponentVersion(ctx context.Context, octx ocm.Context, url, return cv, nil } -func (c *Client) GetProductDescription(ctx context.Context, octx ocm.Context, cv ocm.ComponentVersionAccess) ([]byte, error) { +func (c *Client) GetProductDescription(_ context.Context, _ ocm.Context, cv ocm.ComponentVersionAccess) ([]byte, error) { resources, err := cv.GetResourcesByResourceSelectors(compdesc.ResourceSelectorFunc(func(obj compdesc.ResourceSelectionContext) (bool, error) { return obj.GetType() == ProductDescriptionType, nil })) - if err != nil { return nil, fmt.Errorf("failed to get resource by selector: %w", err) } @@ -165,7 +164,17 @@ func (c *Client) GetResourceData(ctx context.Context, cv ocm.ComponentVersionAcc identities = append(identities, ref.ReferencePath...) identity := ocmmetav1.NewIdentity(ref.Name) - logger.V(4).Info("looking for resource data", "resource", ref.Name, "version", ref.Version, "identities", identities, "identity", identity) + logger.V(v1alpha1.LevelDebug).Info( + "looking for resource data", + "resource", + ref.Name, + "version", + ref.Version, + "identities", + identities, + "identity", + identity, + ) res, _, err := utils.ResolveResourceReference(cv, ocmmetav1.NewNestedResourceRef(identity, identities), cv.Repository()) if err != nil { diff --git a/pkg/validators/gitea/validator.go b/pkg/validators/gitea/validator.go index 0943cfd..9f10cc4 100644 --- a/pkg/validators/gitea/validator.go +++ b/pkg/validators/gitea/validator.go @@ -5,6 +5,7 @@ import ( "fmt" "code.gitea.io/sdk/gitea" + "github.com/open-component-model/mpas-product-controller/api/v1alpha1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -128,10 +129,9 @@ func (v *Validator) createCheckRunStatus(ctx context.Context, repository gitv1al } sha := pr.Head.Sha - logger.V(4).Info("updating SHA", "sha", sha) + logger.V(v1alpha1.LevelDebug).Info("updating SHA", "sha", sha) state, _, err := client.CreateStatus(repository.Spec.Owner, repository.Name, sha, gitea.CreateStatusOption{ - State: status, - //TargetURL: "", + State: status, Description: "MPAS Validation Check", Context: statusCheckName, }) @@ -139,7 +139,8 @@ func (v *Validator) createCheckRunStatus(ctx context.Context, repository gitv1al return fmt.Errorf("failed to create status for pr: %w", err) } - logger.V(4).Info("status", "status", state.State, "id", state.ID, "context", state.Context) + logger.V(v1alpha1.LevelDebug).Info("status", "status", state.State, "id", state.ID, "context", state.Context) + return nil } diff --git a/pkg/validators/github/validator.go b/pkg/validators/github/validator.go index 2cce236..856611d 100644 --- a/pkg/validators/github/validator.go +++ b/pkg/validators/github/validator.go @@ -15,6 +15,7 @@ import ( deliveryv1alpha1 "github.com/open-component-model/git-controller/apis/delivery/v1alpha1" gitv1alpha1 "github.com/open-component-model/git-controller/apis/mpas/v1alpha1" + "github.com/open-component-model/mpas-product-controller/api/v1alpha1" "github.com/open-component-model/mpas-product-controller/pkg/validators" ) @@ -83,7 +84,7 @@ func (v *Validator) IsMergedOrClosed(ctx context.Context, repository gitv1alpha1 } ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: string(token)}) - tc := oauth2.NewClient(context.Background(), ts) + tc := oauth2.NewClient(ctx, ts) g := ggithub.NewClient(tc) @@ -93,6 +94,7 @@ func (v *Validator) IsMergedOrClosed(ctx context.Context, repository gitv1alpha1 } done := pointer.BoolDeref(pr.Merged, false) || pr.ClosedAt != nil + return done, nil } @@ -106,7 +108,7 @@ func (v *Validator) createCheckRunStatus(ctx context.Context, repository gitv1al } ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: string(token)}) - tc := oauth2.NewClient(context.Background(), ts) + tc := oauth2.NewClient(ctx, ts) g := ggithub.NewClient(tc) @@ -116,7 +118,7 @@ func (v *Validator) createCheckRunStatus(ctx context.Context, repository gitv1al } ref := *pr.Head.SHA - logger.V(4).Info("updating SHA", "sha", ref) + logger.V(v1alpha1.LevelDebug).Info("updating SHA", "sha", ref) state, _, err := g.Repositories.CreateStatus(ctx, repository.Spec.Owner, repository.Name, ref, &ggithub.RepoStatus{ // State is the current state of the repository. Possible values are: // pending, success, error, or failure. @@ -128,7 +130,8 @@ func (v *Validator) createCheckRunStatus(ctx context.Context, repository gitv1al return fmt.Errorf("failed to create status for pr: %w", err) } - logger.V(4).Info("status", "status", *state.State, "id", *state.ID, "context", *state.Context) + logger.V(v1alpha1.LevelDebug).Info("status", "status", *state.State, "id", *state.ID, "context", *state.Context) + return nil }