diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 00000000..10c7650b --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,47 @@ +name: codeql + +on: + push: + branches: [ "master" ] + pull_request: + branches: [ "master" ] + schedule: + - cron: '16 16 * * 4' + +jobs: + analyze: + name: Analyze (${{ matrix.language }}) + runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }} + permissions: + security-events: write + packages: read + actions: read + contents: read + + strategy: + fail-fast: false + matrix: + include: + - language: actions + build-mode: none + - language: go + build-mode: autobuild + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Set up Go 1.x + uses: actions/setup-go@v5 + with: + go-version: 1.24 + + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 + with: + category: "/language:${{matrix.language}}" diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 00000000..a034c4fb --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,43 @@ +name: golangci-lint +permissions: + contents: read + pull-requests: write + +on: + pull_request: + push: + branches: [ "master" ] + +env: + GO_VERSION: stable + GOLANGCI_LINT_VERSION: v2.1.1 + +jobs: + detect-modules: + runs-on: ubuntu-latest + outputs: + modules: ${{ steps.set-modules.outputs.modules }} + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + - id: set-modules + run: echo "modules=$(go list -m -json | jq -s '.' | jq -c '[.[].Dir]')" >> $GITHUB_OUTPUT + + golangci-lint: + needs: detect-modules + runs-on: ubuntu-latest + strategy: + matrix: + modules: ${{ fromJSON(needs.detect-modules.outputs.modules) }} + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: ${{ env.GO_VERSION }} + - name: golangci-lint ${{ matrix.modules }} + uses: golangci/golangci-lint-action@v7 + with: + version: ${{ env.GOLANGCI_LINT_VERSION }} + working-directory: ${{ matrix.modules }} diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 4915fc3d..2df44268 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -4,7 +4,7 @@ on: push: branches: [ master ] tags: - - '*' + - "v*.*.*" jobs: push: @@ -21,12 +21,6 @@ jobs: with: go-version: 1.24 - - name: Run golangci-lint - uses: golangci/golangci-lint-action@v3 - with: - version: v1.55.2 - args: --timeout=3m - - name: Test run: | make test @@ -65,6 +59,12 @@ jobs: uses: docker/metadata-action@v5 with: images: ${{ github.repository_owner }}/instance-manager,ghcr.io/${{ github.repository_owner }}/instance-manager + tags: | + type=ref,event=branch + type=ref,event=pr + type=semver,pattern={{version}} + type=semver,pattern={{major}}.{{minor}} + - name: Build and push uses: docker/build-push-action@v6 @@ -72,5 +72,5 @@ jobs: context: . file: ./Dockerfile platforms: linux/amd64,linux/arm/v7,linux/arm64 - push: true + push: ${{ github.event_name != 'pull_request' }} tags: ${{ steps.docker_meta.outputs.tags }} diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml index 079f41ac..8172f50d 100644 --- a/.github/workflows/unit-test.yml +++ b/.github/workflows/unit-test.yml @@ -20,23 +20,9 @@ jobs: - name: Check out code into the Go module directory uses: actions/checkout@v4 - - name: Run golangci-lint - uses: golangci/golangci-lint-action@v3 - with: - version: v1.64.8 - args: --timeout=3m - - - name: Initialize CodeQL - uses: github/codeql-action/init@v3 - with: - languages: go - - name: Build run: | make manager - - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@v3 - name: Test run: | diff --git a/.golangci.yml b/.golangci.yml index bf72a433..d6301eeb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,20 @@ ---- +version: "2" linters: - enable: - - govet - - gosimple - - ineffassign + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + paths: + - third_party$ + - builtin$ + - examples$ +formatters: + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/Makefile b/Makefile index 94d9e23b..6c14168e 100644 --- a/Makefile +++ b/Makefile @@ -34,7 +34,7 @@ all: check-go lint test clean manager # Run tests .PHONY: test -test: generate fmt vet manifests lint +test: generate fmt vet manifests go test ./controllers/... ./api/... -coverprofile coverage.txt .PHONY: bdd @@ -117,6 +117,12 @@ controller-gen: $(CONTROLLER_GEN) ## Download controller-gen locally if necessar $(CONTROLLER_GEN): $(LOCALBIN) GOBIN=$(LOCALBIN) go install sigs.k8s.io/controller-tools/cmd/controller-gen@$(CONTROLLER_GEN_VERSION) +GOLANGCI_LINT_VERSION := v2.1.1 +GOLANGCI_LINT = $(shell pwd)/bin/golangci-lint +.PHONY: golangci-lint +$(GOLANGCI_LINT): $(LOCALBIN) + GOBIN=$(LOCALBIN) go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) + .PHONY: check-go check-go: ifeq ($(GO_VERSION_CHECK),0) @@ -124,9 +130,9 @@ ifeq ($(GO_VERSION_CHECK),0) endif .PHONY: lint -lint: check-go +lint: check-go $(GOLANGCI_LINT) @echo "Running golangci-lint" - golangci-lint run ./... + $(GOLANGCI_LINT) run ./... .PHONY: clean clean: diff --git a/controllers/instancegroup_controller.go b/controllers/instancegroup_controller.go index 5da59d5a..6a4d7346 100644 --- a/controllers/instancegroup_controller.go +++ b/controllers/instancegroup_controller.go @@ -75,13 +75,12 @@ const ( func (r *InstanceGroupReconciler) Finalize(instanceGroup *v1alpha1.InstanceGroup) { // Resource is being deleted - meta := &instanceGroup.ObjectMeta - deletionTimestamp := meta.GetDeletionTimestamp() + deletionTimestamp := instanceGroup.DeletionTimestamp if !deletionTimestamp.IsZero() { // And state is "Deleted" if instanceGroup.GetState() == v1alpha1.ReconcileDeleted { // remove all finalizers - meta.SetFinalizers([]string{}) + instanceGroup.Finalizers = []string{} if err := r.Update(context.Background(), instanceGroup); err != nil { // avoid update errors for already deleted resources if kubeprovider.IsStorageError(err) { @@ -95,11 +94,11 @@ func (r *InstanceGroupReconciler) Finalize(instanceGroup *v1alpha1.InstanceGroup func (r *InstanceGroupReconciler) SetFinalizer(instanceGroup *v1alpha1.InstanceGroup) { // Resource is not being deleted - if instanceGroup.ObjectMeta.DeletionTimestamp.IsZero() { + if instanceGroup.DeletionTimestamp.IsZero() { // And does not contain finalizer - if !common.ContainsString(instanceGroup.ObjectMeta.Finalizers, FinalizerStr) { + if !common.ContainsString(instanceGroup.Finalizers, FinalizerStr) { // Set Finalizer - instanceGroup.ObjectMeta.Finalizers = append(instanceGroup.ObjectMeta.Finalizers, FinalizerStr) + instanceGroup.Finalizers = append(instanceGroup.Finalizers, FinalizerStr) if err := r.Update(context.Background(), instanceGroup); err != nil { r.Log.Error(err, "failed to update custom resource") } @@ -127,7 +126,7 @@ func (r *InstanceGroupReconciler) Reconcile(ctxt context.Context, req ctrl.Reque return ctrl.Result{}, nil } r.Log.Error(err, "reconcile failed", "instancegroup", req.NamespacedName) - r.Metrics.IncFail(req.NamespacedName.String(), ErrorReasonGetFailed) + r.Metrics.IncFail(req.String(), ErrorReasonGetFailed) return ctrl.Result{}, err } statusPatch := kubeprovider.MergePatch(*instanceGroup) diff --git a/controllers/providers/aws/aws.go b/controllers/providers/aws/aws.go index bd8d4f30..7b305c83 100644 --- a/controllers/providers/aws/aws.go +++ b/controllers/providers/aws/aws.go @@ -123,11 +123,8 @@ type AwsWorker struct { } func (w *AwsWorker) WithRetries(f func() bool) error { - var counter int - for { - if counter >= DefaultWaiterRetries { - break - } + counter := 0 + for counter < DefaultWaiterRetries { if f() { return nil } @@ -145,9 +142,10 @@ func (w *AwsWorker) compactTags(tags []map[string]string) map[string]string { value string ) for t, v := range tagSet { - if t == "key" { + switch t { + case "key": key = v - } else if t == "value" { + case "value": value = v } } diff --git a/controllers/providers/aws/eks.go b/controllers/providers/aws/eks.go index 3bf2d361..47ae87e2 100644 --- a/controllers/providers/aws/eks.go +++ b/controllers/providers/aws/eks.go @@ -84,16 +84,16 @@ func (w *AwsWorker) DescribeEKSCluster(clusterName string) (*eks.Cluster, error) } // TODO: Rename - GetNodeGroup -func (w *AwsWorker) GetSelfNodeGroup() (error, *eks.Nodegroup) { +func (w *AwsWorker) GetSelfNodeGroup() (*eks.Nodegroup, error) { input := &eks.DescribeNodegroupInput{ ClusterName: aws.String(w.Parameters["ClusterName"].(string)), NodegroupName: aws.String(w.Parameters["NodegroupName"].(string)), } output, err := w.EksClient.DescribeNodegroup(input) if err != nil { - return err, &eks.Nodegroup{} + return &eks.Nodegroup{}, err } - return nil, output.Nodegroup + return output.Nodegroup, nil } func (w *AwsWorker) DeleteManagedNodeGroup() error { diff --git a/controllers/provisioners/eks/helpers.go b/controllers/provisioners/eks/helpers.go index 2ca592ee..0ec255fb 100644 --- a/controllers/provisioners/eks/helpers.go +++ b/controllers/provisioners/eks/helpers.go @@ -1236,7 +1236,7 @@ func (ctx *EksInstanceGroupContext) GetEksLatestAmi() (string, error) { supportedArchitectures := awsprovider.GetInstanceTypeArchitectures(state.GetInstanceTypeInfo(), configuration.InstanceType) arch := FilterSupportedArch(supportedArchitectures) if arch == "" { - return "", fmt.Errorf("No supported CPU architecture found for instance type %s", configuration.InstanceType) + return "", fmt.Errorf("no supported CPU architecture found for instance type %s", configuration.InstanceType) } return ctx.AwsWorker.GetEksLatestAmi(OSFamily, arch, clusterVersion) @@ -1254,7 +1254,7 @@ func (ctx *EksInstanceGroupContext) GetEksSsmAmi(id string) (string, error) { supportedArchitectures := awsprovider.GetInstanceTypeArchitectures(state.GetInstanceTypeInfo(), configuration.InstanceType) arch := FilterSupportedArch(supportedArchitectures) if arch == "" { - return "", fmt.Errorf("No supported CPU architecture found for instance type %s", configuration.InstanceType) + return "", fmt.Errorf("no supported CPU architecture found for instance type %s", configuration.InstanceType) } return ctx.AwsWorker.GetEksSsmAmi(osFamily, arch, clusterVersion, id) diff --git a/controllers/provisioners/eks/helpers_test.go b/controllers/provisioners/eks/helpers_test.go index a0656ae5..07838a31 100644 --- a/controllers/provisioners/eks/helpers_test.go +++ b/controllers/provisioners/eks/helpers_test.go @@ -1272,7 +1272,7 @@ func TestGetEksLatestAmi(t *testing.T) { name: "AmazonLinux2-noarch", OSFamily: "amazonlinux2", arch: "noarch", - expectedError: fmt.Errorf("No supported CPU architecture found for instance type %s", instanceType), + expectedError: fmt.Errorf("no supported CPU architecture found for instance type %s", instanceType), }, } diff --git a/controllers/provisioners/eks/state.go b/controllers/provisioners/eks/state.go index 8e724891..aadf2fb4 100644 --- a/controllers/provisioners/eks/state.go +++ b/controllers/provisioners/eks/state.go @@ -37,7 +37,7 @@ func (ctx *EksInstanceGroupContext) StateDiscovery() { } var deleted bool - if !ctx.InstanceGroup.ObjectMeta.DeletionTimestamp.IsZero() { + if !ctx.InstanceGroup.DeletionTimestamp.IsZero() { deleted = true } diff --git a/controllers/provisioners/eksfargate/eksfargate.go b/controllers/provisioners/eksfargate/eksfargate.go index 096fc6e2..0bea47a7 100644 --- a/controllers/provisioners/eksfargate/eksfargate.go +++ b/controllers/provisioners/eksfargate/eksfargate.go @@ -326,7 +326,7 @@ func (ctx *FargateInstanceGroupContext) StateDiscovery() { instanceGroup := ctx.GetInstanceGroup() if instanceGroup.GetState() == v1alpha1.ReconcileInit { - if instanceGroup.ObjectMeta.DeletionTimestamp.IsZero() { + if instanceGroup.GetObjectMeta().GetDeletionTimestamp().IsZero() { if ctx.GetDiscoveredState().IsProvisioned() { // Role exists and the Profile exists in some form (creating) if awsprovider.IsProfileInConditionState(ctx.GetDiscoveredState().GetProfileStatus(), OngoingStateString) { diff --git a/controllers/provisioners/eksmanaged/eksmanaged.go b/controllers/provisioners/eksmanaged/eksmanaged.go index 0fe4778d..13ded724 100644 --- a/controllers/provisioners/eksmanaged/eksmanaged.go +++ b/controllers/provisioners/eksmanaged/eksmanaged.go @@ -45,7 +45,7 @@ func (ctx *EksManagedInstanceGroupContext) CloudDiscovery() error { if provisioned { discoveredState.SetProvisioned(true) - err, createdResource := ctx.AwsWorker.GetSelfNodeGroup() + createdResource, err := ctx.AwsWorker.GetSelfNodeGroup() if err != nil { return err } @@ -80,25 +80,7 @@ func (ctx *EksManagedInstanceGroupContext) StateDiscovery() { ) if instanceGroup.GetState() == v1alpha1.ReconcileInit { - if ctx.InstanceGroup.ObjectMeta.DeletionTimestamp.IsZero() { - // resource is not being deleted - if provisioned { - // nodegroup exists - if awsprovider.IsNodeGroupInConditionState(nodeGroupState, OngoingStateString) { - // nodegroup is in an ongoing state - instanceGroup.SetState(v1alpha1.ReconcileModifying) - } else if awsprovider.IsNodeGroupInConditionState(nodeGroupState, FiniteStateString) { - // nodegroup is in a finite state - instanceGroup.SetState(v1alpha1.ReconcileInitUpdate) - } else if awsprovider.IsNodeGroupInConditionState(nodeGroupState, UnrecoverableErrorString) { - // nodegroup is in unrecoverable error state - instanceGroup.SetState(v1alpha1.ReconcileErr) - } - } else { - // nodegroup does not exist - instanceGroup.SetState(v1alpha1.ReconcileInitCreate) - } - } else { + if !instanceGroup.DeletionTimestamp.IsZero() { // resource is being deleted if provisioned { if awsprovider.IsNodeGroupInConditionState(nodeGroupState, OngoingStateString) { @@ -118,6 +100,24 @@ func (ctx *EksManagedInstanceGroupContext) StateDiscovery() { // Stack does not exist instanceGroup.SetState(v1alpha1.ReconcileDeleted) } + } else { + // resource is not being deleted + if provisioned { + // nodegroup exists + if awsprovider.IsNodeGroupInConditionState(nodeGroupState, OngoingStateString) { + // nodegroup is in an ongoing state + instanceGroup.SetState(v1alpha1.ReconcileModifying) + } else if awsprovider.IsNodeGroupInConditionState(nodeGroupState, FiniteStateString) { + // nodegroup is in a finite state + instanceGroup.SetState(v1alpha1.ReconcileInitUpdate) + } else if awsprovider.IsNodeGroupInConditionState(nodeGroupState, UnrecoverableErrorString) { + // nodegroup is in unrecoverable error state + instanceGroup.SetState(v1alpha1.ReconcileErr) + } + } else { + // nodegroup does not exist + instanceGroup.SetState(v1alpha1.ReconcileInitCreate) + } } } }