From 63a0a69aa0db2d6d1540ba762885734ad83c7f6c Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Fri, 24 Nov 2023 12:56:55 +0100 Subject: [PATCH] feat: applying lint and fixing the lint issues (#86) --- .github/workflows/tests.yaml | 4 +- .golangci.yaml | 120 ++++++++++++++++++ Makefile | 11 ++ apis/delivery/v1alpha1/condition_types.go | 6 - apis/delivery/v1alpha1/constants.go | 4 + apis/delivery/v1alpha1/groupversion_info.go | 4 +- apis/delivery/v1alpha1/sync_types.go | 8 +- apis/mpas/v1alpha1/groupversion_info.go | 4 +- apis/mpas/v1alpha1/repository_types.go | 8 +- .../bases/delivery.ocm.software_syncs.yaml | 6 +- .../bases/mpas.ocm.software_repositories.yaml | 6 +- controllers/delivery/sync_controller.go | 102 ++++++++------- controllers/mpas/repository_controller.go | 20 +-- main.go | 3 +- pkg/event/event.go | 1 + pkg/gogit/git.go | 21 ++- pkg/gogit/tar.go | 5 +- pkg/providers/gitea/gitea.go | 4 +- pkg/providers/github/github.go | 4 +- pkg/providers/gitlab/gitlab.go | 8 +- pkg/providers/gogit/gogit.go | 24 +++- pkg/providers/providers.go | 2 +- 22 files changed, 281 insertions(+), 94 deletions(-) create mode 100644 .golangci.yaml diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 363cd76..a9a80aa 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -1,4 +1,4 @@ -name: tests +name: tests and linting 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..d28dfeb --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,120 @@ +run: + go: "1.21" + timeout: 10m + tests: false + allow-parallel-runners: true + skip-dirs: + - "./*/mock" + skip-files: + - "pkg/providers/fakes/fake_provider.go" + +linters-settings: + funlen: + lines: 150 + statements: 60 + staticcheck: + go: "1.21" + stylecheck: + go: "1.21" + cyclop: + max-complexity: 20 + skip-tests: true + gosec: + exclude-generated: true + lll: + line-length: 150 + misspell: + locale: US + govet: + check-shadowing: true + 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 4fd1d50..d0e4420 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 ./... -coverprofile cover.out @@ -137,6 +141,7 @@ $(LOCALBIN): mkdir -p $(LOCALBIN) ## Tool Binaries +GOLANGCI_LINT ?= $(LOCALBIN)/golangci-lint KUSTOMIZE ?= $(LOCALBIN)/kustomize CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen ENVTEST ?= $(LOCALBIN)/setup-envtest @@ -146,6 +151,7 @@ GEN_CRD_API_REFERENCE_DOCS ?= $(LOCALBIN)/gen-crd-api-reference-docs KUSTOMIZE_VERSION ?= v3.8.7 CONTROLLER_TOOLS_VERSION ?= v0.9.2 GEN_API_REF_DOCS_VERSION ?= e327d0730470cbd61b06300f81c5fcf91c23c113 +GOLANGCI_LINT_VERSION ?= v1.55.2 KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh" .PHONY: kustomize @@ -178,3 +184,8 @@ generate-license: gen-crd-api-reference-docs: $(GEN_CRD_API_REFERENCE_DOCS) $(GEN_CRD_API_REFERENCE_DOCS): $(LOCALBIN) GOBIN=$(LOCALBIN) go install github.com/ahmetb/gen-crd-api-reference-docs@$(GEN_API_REF_DOCS_VERSION) + +.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/apis/delivery/v1alpha1/condition_types.go b/apis/delivery/v1alpha1/condition_types.go index e18cbf9..749293a 100644 --- a/apis/delivery/v1alpha1/condition_types.go +++ b/apis/delivery/v1alpha1/condition_types.go @@ -5,9 +5,6 @@ package v1alpha1 const ( - // PatchFailedReason is used when we couldn't patch an object. - PatchFailedReason = "PatchFailed" - // SnapshotGetFailedReason is used when the needed snapshot does not exist. SnapshotGetFailedReason = "SnapshotGetFailed" @@ -22,7 +19,4 @@ const ( // CreatePullRequestFailedReason is used when creating a pull request failed. CreatePullRequestFailedReason = "CreatePullRequestFailed" - - // GitRepositoryCreateFailedReason is used when creating a git repository failed. - GitRepositoryCreateFailedReason = "GitRepositoryCreateFailed" ) diff --git a/apis/delivery/v1alpha1/constants.go b/apis/delivery/v1alpha1/constants.go index 2ccd19c..72103d7 100644 --- a/apis/delivery/v1alpha1/constants.go +++ b/apis/delivery/v1alpha1/constants.go @@ -4,3 +4,7 @@ const ( // StatusCheckName defines the name of the check a PullRequest will have. StatusCheckName = "mpas/validation-check" ) + +const ( + LevelDebug = 4 +) diff --git a/apis/delivery/v1alpha1/groupversion_info.go b/apis/delivery/v1alpha1/groupversion_info.go index c5d95a1..1033a2f 100644 --- a/apis/delivery/v1alpha1/groupversion_info.go +++ b/apis/delivery/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: "delivery.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/apis/delivery/v1alpha1/sync_types.go b/apis/delivery/v1alpha1/sync_types.go index cbb9561..6b55667 100644 --- a/apis/delivery/v1alpha1/sync_types.go +++ b/apis/delivery/v1alpha1/sync_types.go @@ -33,7 +33,7 @@ type PullRequestTemplate struct { Base string `json:"base,omitempty"` } -// SyncSpec defines the desired state of Sync +// SyncSpec defines the desired state of Sync. type SyncSpec struct { SnapshotRef v1.LocalObjectReference `json:"snapshotRef"` RepositoryRef meta.NamespacedObjectReference `json:"repositoryRef"` @@ -48,7 +48,7 @@ type SyncSpec struct { PullRequestTemplate PullRequestTemplate `json:"pullRequestTemplate,omitempty"` } -// SyncStatus defines the observed state of Sync +// SyncStatus defines the observed state of Sync. type SyncStatus struct { Digest string `json:"digest,omitempty"` @@ -95,7 +95,7 @@ func (in Sync) GetRequeueAfter() time.Duration { //+kubebuilder:object:root=true //+kubebuilder:subresource:status -// Sync is the Schema for the syncs API +// Sync is the Schema for the syncs API. type Sync struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -106,7 +106,7 @@ type Sync struct { //+kubebuilder:object:root=true -// SyncList contains a list of Sync +// SyncList contains a list of Sync. type SyncList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/apis/mpas/v1alpha1/groupversion_info.go b/apis/mpas/v1alpha1/groupversion_info.go index 8ed463d..2e254dc 100644 --- a/apis/mpas/v1alpha1/groupversion_info.go +++ b/apis/mpas/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/apis/mpas/v1alpha1/repository_types.go b/apis/mpas/v1alpha1/repository_types.go index 8b5fca9..ed6faa6 100644 --- a/apis/mpas/v1alpha1/repository_types.go +++ b/apis/mpas/v1alpha1/repository_types.go @@ -28,7 +28,7 @@ var ( ExistingRepositoryPolicyFail ExistingRepositoryPolicy = "fail" ) -// RepositorySpec defines the desired state of Repository +// RepositorySpec defines the desired state of Repository. type RepositorySpec struct { //+required Provider string `json:"provider"` @@ -73,7 +73,7 @@ type CommitTemplate struct { Name string `json:"name"` } -// RepositoryStatus defines the observed state of Repository +// RepositoryStatus defines the observed state of Repository. type RepositoryStatus struct { // ObservedGeneration is the last reconciled generation. // +optional @@ -143,7 +143,7 @@ func (in *Repository) SetObservedGeneration(v int64) { //+kubebuilder:object:root=true //+kubebuilder:subresource:status -// Repository is the Schema for the repositories API +// Repository is the Schema for the repositories API. type Repository struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` @@ -154,7 +154,7 @@ type Repository struct { //+kubebuilder:object:root=true -// RepositoryList contains a list of Repository +// RepositoryList contains a list of Repository. type RepositoryList struct { metav1.TypeMeta `json:",inline"` metav1.ListMeta `json:"metadata,omitempty"` diff --git a/config/crd/bases/delivery.ocm.software_syncs.yaml b/config/crd/bases/delivery.ocm.software_syncs.yaml index 5bdc117..e0420bf 100644 --- a/config/crd/bases/delivery.ocm.software_syncs.yaml +++ b/config/crd/bases/delivery.ocm.software_syncs.yaml @@ -18,7 +18,7 @@ spec: - name: v1alpha1 schema: openAPIV3Schema: - description: Sync is the Schema for the syncs API + description: Sync is the Schema for the syncs API. properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation @@ -33,7 +33,7 @@ spec: metadata: type: object spec: - description: SyncSpec defines the desired state of Sync + description: SyncSpec defines the desired state of Sync. properties: automaticPullRequestCreation: type: boolean @@ -106,7 +106,7 @@ spec: - subPath type: object status: - description: SyncStatus defines the observed state of Sync + description: SyncStatus defines the observed state of Sync. properties: conditions: items: diff --git a/config/crd/bases/mpas.ocm.software_repositories.yaml b/config/crd/bases/mpas.ocm.software_repositories.yaml index 663feac..bfacfd0 100644 --- a/config/crd/bases/mpas.ocm.software_repositories.yaml +++ b/config/crd/bases/mpas.ocm.software_repositories.yaml @@ -18,7 +18,7 @@ spec: - name: v1alpha1 schema: openAPIV3Schema: - description: Repository is the Schema for the repositories API + description: Repository is the Schema for the repositories API. properties: apiVersion: description: 'APIVersion defines the versioned schema of this representation @@ -33,7 +33,7 @@ spec: metadata: type: object spec: - description: RepositorySpec defines the desired state of Repository + description: RepositorySpec defines the desired state of Repository. properties: commitTemplate: description: CommitTemplate defines the commit template to use when @@ -113,7 +113,7 @@ spec: - provider type: object status: - description: RepositoryStatus defines the observed state of Repository + description: RepositoryStatus defines the observed state of Repository. properties: conditions: items: diff --git a/controllers/delivery/sync_controller.go b/controllers/delivery/sync_controller.go index bde50ba..9198e14 100644 --- a/controllers/delivery/sync_controller.go +++ b/controllers/delivery/sync_controller.go @@ -13,7 +13,6 @@ import ( "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/patch" rreconcile "github.com/fluxcd/pkg/runtime/reconcile" - "github.com/open-component-model/ocm-controller/pkg/status" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -25,6 +24,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" ocmv1 "github.com/open-component-model/ocm-controller/api/v1alpha1" + "github.com/open-component-model/ocm-controller/pkg/status" "github.com/open-component-model/git-controller/apis/delivery/v1alpha1" mpasv1alpha1 "github.com/open-component-model/git-controller/apis/mpas/v1alpha1" @@ -32,7 +32,7 @@ import ( "github.com/open-component-model/git-controller/pkg/providers" ) -// SyncReconciler reconciles a Sync object +// SyncReconciler reconciles a Sync object. type SyncReconciler struct { client.Client kuberecorder.EventRecorder @@ -84,13 +84,49 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctr // Should only be deleted on a success. rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress for resource: %s", obj.Name) - // it's important that this happens here so any residual status condition can be overwritten / set. + // It's important that this happens here so any residual status condition can be overwritten / set. if obj.Status.Digest != "" { status.MarkReady(r.EventRecorder, obj, "Digest already reconciled") return ctrl.Result{}, nil } + if err := r.reconcile(ctx, obj); err != nil { + return ctrl.Result{}, err + } + + return ctrl.Result{}, nil +} + +// SetupWithManager sets up the controller with the Manager. +func (r *SyncReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&v1alpha1.Sync{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). + Complete(r) +} + +func (r *SyncReconciler) parseAuthSecret(secret *corev1.Secret, opts *pkg.PushOptions) { + if _, ok := secret.Data["identity"]; ok { + opts.Auth = &pkg.Auth{ + SSH: &pkg.SSH{ + PemBytes: secret.Data["identity"], + User: string(secret.Data["username"]), + Password: string(secret.Data["password"]), + }, + } + + return + } + // default to basic auth. + opts.Auth = &pkg.Auth{ + BasicAuth: &pkg.BasicAuth{ + Username: string(secret.Data["username"]), + Password: string(secret.Data["password"]), + }, + } +} + +func (r *SyncReconciler) reconcile(ctx context.Context, obj *v1alpha1.Sync) error { if obj.Generation != obj.Status.ObservedGeneration { rreconcile.ProgressiveStatus( false, @@ -103,14 +139,14 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctr } snapshot := &ocmv1.Snapshot{} - if err = r.Get(ctx, types.NamespacedName{ + if err := r.Get(ctx, types.NamespacedName{ Namespace: obj.Namespace, Name: obj.Spec.SnapshotRef.Name, }, snapshot); err != nil { err = fmt.Errorf("failed to find snapshot: %w", err) status.MarkNotReady(r.EventRecorder, obj, v1alpha1.SnapshotGetFailedReason, err.Error()) - return ctrl.Result{}, err + return err } namespace := obj.Spec.RepositoryRef.Namespace @@ -119,25 +155,25 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctr } repository := &mpasv1alpha1.Repository{} - if err = r.Get(ctx, types.NamespacedName{ + if err := r.Get(ctx, types.NamespacedName{ Namespace: namespace, Name: obj.Spec.RepositoryRef.Name, }, repository); err != nil { err = fmt.Errorf("failed to find repository: %w", err) status.MarkNotReady(r.EventRecorder, obj, v1alpha1.RepositoryGetFailedReason, err.Error()) - return ctrl.Result{}, err + return err } authSecret := &corev1.Secret{} - if err = r.Get(ctx, types.NamespacedName{ + if err := r.Get(ctx, types.NamespacedName{ Namespace: repository.Namespace, Name: repository.Spec.Credentials.SecretRef.Name, }, authSecret); err != nil { err = fmt.Errorf("failed to find authentication secret: %w", err) status.MarkNotReady(r.EventRecorder, obj, v1alpha1.CredentialsNotFoundReason, err.Error()) - return ctrl.Result{}, err + return err } baseBranch := obj.Spec.CommitTemplate.BaseBranch @@ -149,13 +185,20 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctr if targetBranch == "" && obj.Spec.AutomaticPullRequestCreation { targetBranch = fmt.Sprintf("branch-%d", time.Now().Unix()) } else if targetBranch == "" && !obj.Spec.AutomaticPullRequestCreation { - err = fmt.Errorf("branch cannot be empty if automatic pull request creation is not enabled") + err := fmt.Errorf("branch cannot be empty if automatic pull request creation is not enabled") status.MarkNotReady(r.EventRecorder, obj, v1alpha1.GitRepositoryPushFailedReason, err.Error()) - return ctrl.Result{}, err + return err } - rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "preparing to push snapshot content with base branch %s and target %s", baseBranch, targetBranch) + rreconcile.ProgressiveStatus( + false, + obj, + meta.ProgressingReason, + "preparing to push snapshot content with base branch %s and target %s", + baseBranch, + targetBranch, + ) opts := &pkg.PushOptions{ URL: repository.GetRepositoryURL(), @@ -171,12 +214,12 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctr r.parseAuthSecret(authSecret, opts) var digest string - digest, err = r.Git.Push(ctx, opts) + digest, err := r.Git.Push(ctx, opts) if err != nil { err = fmt.Errorf("failed to push to git repository: %w", err) status.MarkNotReady(r.EventRecorder, obj, v1alpha1.GitRepositoryPushFailedReason, err.Error()) - return ctrl.Result{}, err + return err } obj.Status.Digest = digest @@ -189,7 +232,7 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctr err = fmt.Errorf("failed to create pull request: %w", err) status.MarkNotReady(r.EventRecorder, obj, v1alpha1.CreatePullRequestFailedReason, err.Error()) - return ctrl.Result{}, err + return err } obj.Status.PullRequestID = id @@ -197,32 +240,5 @@ func (r *SyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctr status.MarkReady(r.EventRecorder, obj, "Reconciliation success") - return ctrl.Result{}, nil -} - -// SetupWithManager sets up the controller with the Manager. -func (r *SyncReconciler) SetupWithManager(mgr ctrl.Manager) error { - return ctrl.NewControllerManagedBy(mgr). - For(&v1alpha1.Sync{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})). - Complete(r) -} - -func (r *SyncReconciler) parseAuthSecret(secret *corev1.Secret, opts *pkg.PushOptions) { - if _, ok := secret.Data["identity"]; ok { - opts.Auth = &pkg.Auth{ - SSH: &pkg.SSH{ - PemBytes: secret.Data["identity"], - User: string(secret.Data["username"]), - Password: string(secret.Data["password"]), - }, - } - return - } - // default to basic auth. - opts.Auth = &pkg.Auth{ - BasicAuth: &pkg.BasicAuth{ - Username: string(secret.Data["username"]), - Password: string(secret.Data["password"]), - }, - } + return nil } diff --git a/controllers/mpas/repository_controller.go b/controllers/mpas/repository_controller.go index 0a34136..724ecd2 100644 --- a/controllers/mpas/repository_controller.go +++ b/controllers/mpas/repository_controller.go @@ -25,7 +25,7 @@ import ( "github.com/open-component-model/git-controller/pkg/providers" ) -// RepositoryReconciler reconciles a Repository object +// RepositoryReconciler reconciles a Repository object. type RepositoryReconciler struct { client.Client kuberecorder.EventRecorder @@ -69,7 +69,11 @@ func (r *RepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Request) // Should only be deleted on a success. rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress for resource: %s", obj.Name) - return r.reconcile(ctx, obj) + if err := r.reconcile(ctx, obj); err != nil { + return ctrl.Result{}, err + } + + return ctrl.Result{}, nil } // SetupWithManager sets up the controller with the Manager. @@ -79,7 +83,7 @@ func (r *RepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } -func (r *RepositoryReconciler) reconcile(ctx context.Context, obj *mpasv1alpha1.Repository) (ctrl.Result, error) { +func (r *RepositoryReconciler) reconcile(ctx context.Context, obj *mpasv1alpha1.Repository) error { if obj.Generation != obj.Status.ObservedGeneration { rreconcile.ProgressiveStatus( false, @@ -97,26 +101,26 @@ func (r *RepositoryReconciler) reconcile(ctx context.Context, obj *mpasv1alpha1. err := fmt.Errorf("failed to create repository: %w", err) status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.RepositoryCreateFailedReason, err.Error()) - return ctrl.Result{}, err + return err } rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "setting up branch protection rules: %s", obj.Name) if err := r.Provider.CreateBranchProtection(ctx, *obj); err != nil { - if errors.Is(err, providers.NotSupportedError) { + if errors.Is(err, providers.ErrNotSupported) { status.MarkReady(r.EventRecorder, obj, "Successful reconciliation") // ignore and return without branch protection rules. - return ctrl.Result{}, nil + return nil } err := fmt.Errorf("failed to update branch protection rules: %w", err) status.MarkNotReady(r.EventRecorder, obj, mpasv1alpha1.UpdatingBranchProtectionFailedReason, err.Error()) - return ctrl.Result{}, err + return err } status.MarkReady(r.EventRecorder, obj, "Successful reconciliation") - return ctrl.Result{}, nil + return nil } diff --git a/main.go b/main.go index ef21e0c..2432ff1 100644 --- a/main.go +++ b/main.go @@ -76,10 +76,11 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + const metricsServerPort = 9443 mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, MetricsBindAddress: metricsAddr, - Port: 9443, + Port: metricsServerPort, HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "76b5aa10.ocm.software", diff --git a/pkg/event/event.go b/pkg/event/event.go index 76dc4fa..15615fb 100644 --- a/pkg/event/event.go +++ b/pkg/event/event.go @@ -13,6 +13,7 @@ import ( kuberecorder "k8s.io/client-go/tools/record" ) +// New creates a new event recorder. func New(recorder kuberecorder.EventRecorder, obj conditions.Getter, severity, msg string, metadata map[string]string) { if metadata == nil { metadata = map[string]string{} diff --git a/pkg/gogit/git.go b/pkg/gogit/git.go index 45b7c92..4bc4b21 100644 --- a/pkg/gogit/git.go +++ b/pkg/gogit/git.go @@ -19,6 +19,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/transport/http" "github.com/go-git/go-git/v5/plumbing/transport/ssh" "github.com/go-logr/logr" + "github.com/open-component-model/git-controller/apis/delivery/v1alpha1" "github.com/open-component-model/ocm-controller/pkg/cache" "github.com/open-component-model/ocm-controller/pkg/ocm" @@ -39,11 +40,21 @@ func NewGoGit(log logr.Logger, cache cache.Cache) *Git { } func (g *Git) Push(ctx context.Context, opts *pkg.PushOptions) (string, error) { - g.Logger.V(4).Info("running push operation", "msg", opts.Message, "snapshot", opts.Snapshot.Name, "url", opts.URL, "sub-path", opts.SubPath) + g.Logger.V(v1alpha1.LevelDebug).Info( + "running push operation", + "msg", + opts.Message, + "snapshot", + opts.Snapshot.Name, + "url", + opts.URL, + "sub-path", + opts.SubPath, + ) dir, err := os.MkdirTemp("", "clone") if err != nil { - return "", fmt.Errorf("failed to initialise temp folder: %w", err) + return "", fmt.Errorf("failed to initialize temp folder: %w", err) } var auth transport.AuthMethod @@ -90,7 +101,8 @@ func (g *Git) Push(ctx context.Context, opts *pkg.PushOptions) (string, error) { } dir = filepath.Join(dir, opts.SubPath) - if err := os.MkdirAll(dir, 0777); err != nil { + const perm = 0o777 + if err := os.MkdirAll(dir, perm); err != nil { return "", fmt.Errorf("failed to create subPath: %w", err) } @@ -133,7 +145,7 @@ func (g *Git) Push(ctx context.Context, opts *pkg.PushOptions) (string, error) { if err != nil { return "", fmt.Errorf("failed to commit changes: %w", err) } - g.Logger.V(4).Info("pushing commit", "commit", commit) + g.Logger.V(v1alpha1.LevelDebug).Info("pushing commit", "commit", commit) pushOptions := &git.PushOptions{ Prune: opts.Prune, Auth: auth, @@ -141,5 +153,6 @@ func (g *Git) Push(ctx context.Context, opts *pkg.PushOptions) (string, error) { if err := r.Push(pushOptions); err != nil { return "", fmt.Errorf("failed to push new snapshot: %w", err) } + return opts.Snapshot.Spec.Digest, nil } diff --git a/pkg/gogit/tar.go b/pkg/gogit/tar.go index ef08df5..49684ef 100644 --- a/pkg/gogit/tar.go +++ b/pkg/gogit/tar.go @@ -18,10 +18,11 @@ func Untar(in io.Reader, dir string) error { if errors.Is(err, io.EOF) { return nil } + return err } - abs := filepath.Join(dir, header.Name) + abs := filepath.Join(dir, header.Name) //nolint:gosec // tar switch header.Typeflag { case tar.TypeDir: @@ -37,7 +38,7 @@ func Untar(in io.Reader, dir string) error { // archive can be an image layer and that can even reach the gigabyte range. // For now, we acknowledge the risk. // - // We checked other softwares and tried to figure out how they manage this, + // We checked other software and tried to figure out how they manage this, // but it's handled the same way. if _, err := io.Copy(file, tr); err != nil { return fmt.Errorf("unable to copy tar file to filesystem: %w", err) diff --git a/pkg/providers/gitea/gitea.go b/pkg/providers/gitea/gitea.go index d9b4461..7dc67b6 100644 --- a/pkg/providers/gitea/gitea.go +++ b/pkg/providers/gitea/gitea.go @@ -221,7 +221,8 @@ func (c *Client) CreateBranchProtection(ctx context.Context, repository mpasv1al return c.next.CreateBranchProtection(ctx, repository) } - //TODO: use safe auth strategy post MVP + //nolint:godox // ignore todo + // TODO: use safe auth strategy post MVP secret := &v1.Secret{} if err := c.client.Get(ctx, types.NamespacedName{ Name: repository.Spec.Credentials.SecretRef.Name, @@ -277,5 +278,6 @@ func (c *Client) getDomain(obj mpasv1alpha1.Repository) (string, error) { // construct the domain including the scheme and host but without the path // gitea requires a host and a scheme domain := fmt.Sprintf("%s://%s", u.Scheme, u.Host) + return domain, nil } diff --git a/pkg/providers/github/github.go b/pkg/providers/github/github.go index 6ee276a..c93520c 100644 --- a/pkg/providers/github/github.go +++ b/pkg/providers/github/github.go @@ -90,7 +90,7 @@ func (c *Client) CreateBranchProtection(ctx context.Context, obj mpasv1alpha1.Re } ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: string(token)}) - tc := oauth2.NewClient(context.Background(), ts) + tc := oauth2.NewClient(ctx, ts) g := ggithub.NewClient(tc) if _, _, err := g.Repositories.UpdateBranchProtection(ctx, obj.Spec.Owner, obj.Name, obj.Spec.DefaultBranch, &ggithub.ProtectionRequest{ @@ -188,7 +188,7 @@ func (c *Client) createCheckRun(ctx context.Context, repository mpasv1alpha1.Rep } ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: string(token)}) - tc := oauth2.NewClient(context.Background(), ts) + tc := oauth2.NewClient(ctx, ts) g := ggithub.NewClient(tc) // diff --git a/pkg/providers/gitlab/gitlab.go b/pkg/providers/gitlab/gitlab.go index 48ffe6a..c1712b1 100644 --- a/pkg/providers/gitlab/gitlab.go +++ b/pkg/providers/gitlab/gitlab.go @@ -26,10 +26,8 @@ const ( defaultDomain = gitlab.DefaultDomain ) -var ( - // for now, only personal tokens are supported - tokenType = "personal" -) +// tokenType for now, only personal tokens are supported. +var tokenType = "personal" // Client gitlab. type Client struct { @@ -134,5 +132,5 @@ func (c *Client) CreateBranchProtection(ctx context.Context, obj mpasv1alpha1.Re return c.next.CreateBranchProtection(ctx, obj) } - return providers.NotSupportedError + return providers.ErrNotSupported } diff --git a/pkg/providers/gogit/gogit.go b/pkg/providers/gogit/gogit.go index 0c9324a..9ece7f3 100644 --- a/pkg/providers/gogit/gogit.go +++ b/pkg/providers/gogit/gogit.go @@ -19,6 +19,8 @@ import ( ) // CreateOrganizationRepository creates a repository for an authenticated organization. +// +//nolint:dupl // unfortunately it is the same but using a different interface with different parameter types. func CreateOrganizationRepository(ctx context.Context, gc gitprovider.Client, domain string, obj mpasv1alpha1.Repository) error { logger := log.FromContext(ctx) @@ -88,6 +90,8 @@ func CreateOrganizationRepository(ctx context.Context, gc gitprovider.Client, do } // CreateUserRepository creates a repository for an authenticated user. +// +//nolint:dupl // unfortunately it is the same but using a different interface with different parameter types. func CreateUserRepository(ctx context.Context, gc gitprovider.Client, domain string, obj mpasv1alpha1.Repository) error { logger := log.FromContext(ctx) @@ -157,7 +161,15 @@ func CreateUserRepository(ctx context.Context, gc gitprovider.Client, domain str } // CreateOrganizationPullRequest creates a pull-request for an organization owned repository. -func CreateOrganizationPullRequest(ctx context.Context, gc gitprovider.Client, domain, branch string, spec deliveryv1alpha1.PullRequestTemplate, repository mpasv1alpha1.Repository) (int, error) { +// +//nolint:dupl // unfortunately it is the same but using a different interface with different parameter types. +func CreateOrganizationPullRequest( + ctx context.Context, + gc gitprovider.Client, + domain, branch string, + spec deliveryv1alpha1.PullRequestTemplate, + repository mpasv1alpha1.Repository, +) (int, error) { // find the repository repo, err := gc.OrgRepositories().Get(ctx, gitprovider.OrgRepositoryRef{ OrganizationRef: gitprovider.OrganizationRef{ @@ -200,7 +212,15 @@ func CreateOrganizationPullRequest(ctx context.Context, gc gitprovider.Client, d } // CreateUserPullRequest creates a pull-request for a user owned repository. -func CreateUserPullRequest(ctx context.Context, gc gitprovider.Client, domain, branch string, spec deliveryv1alpha1.PullRequestTemplate, repository mpasv1alpha1.Repository) (int, error) { +// +//nolint:dupl // unfortunately it is the same but using a different interface with different parameter types. +func CreateUserPullRequest( + ctx context.Context, + gc gitprovider.Client, + domain, branch string, + spec deliveryv1alpha1.PullRequestTemplate, + repository mpasv1alpha1.Repository, +) (int, error) { // find the repository repo, err := gc.UserRepositories().Get(ctx, gitprovider.UserRepositoryRef{ UserRef: gitprovider.UserRef{ diff --git a/pkg/providers/providers.go b/pkg/providers/providers.go index 75169ec..b41dd99 100644 --- a/pkg/providers/providers.go +++ b/pkg/providers/providers.go @@ -18,7 +18,7 @@ const ( DefaultDescription = "Pull requested created automatically by OCM Git Controller." ) -var NotSupportedError = errors.New("functionality not supported by provider") +var ErrNotSupported = errors.New("functionality not supported by provider") // Provider adds the ability to create repositories and pull requests. type Provider interface {