Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Maintenance #64

Merged
merged 7 commits into from
Jan 21, 2025
Merged

Maintenance #64

merged 7 commits into from
Jan 21, 2025

Conversation

hnnsgstfssn
Copy link
Contributor

@hnnsgstfssn hnnsgstfssn commented Jan 20, 2025

This pull request includes several updates and improvements across different files, focusing on dependency updates, workflow configurations, and code refactoring. The most important changes are summarized below:

Dependency Updates:

  • Updated various dependencies in the go.mod file to their latest versions, including github.com/argoproj/argo-cd, github.com/cenkalti/backoff, and google.golang.org/grpc. [1] [2] [3] [4] [5]

Workflow Configurations:

  • Modified the .github/workflows/lint.yml file to use go-version-file instead of a hardcoded Go version and reordered steps for better clarity.

Code Refactoring:

  • Replaced the exportloopref linter with copyloopvar in the .golangci.yml file.
  • Refactored test files to remove redundant variable reassignments within loop constructs. [1] [2] [3] [4] [5]
  • Updated Makefile to use go mod download instead of go mod vendor for dependency management.

Mock Generation:

  • Updated internal/pkg/mocks/mocks.go to use go.uber.org/mock for generating mock files, replacing github.com/golang/mock.

Function Enhancements:

  • Enhanced the SetAppInstance function call in internal/pkg/argocd/argocd_copied_from_upstream.go to include GetInstallationID.
  • Updated imports in internal/pkg/argocd/argocd_test.go to use go.uber.org/mock/gomock.

The above is a generated description of the changes.

@hnnsgstfssn hnnsgstfssn requested a review from a team as a code owner January 20, 2025 18:09
Base automatically changed from tidy to main January 21, 2025 12:35
This prevents the following error during setup step caused by missing
dependency files.

    Warning: Restore cache failed: Dependencies file is not found in
    /home/runner/work/telefonistka/telefonistka. Supported file pattern:
    go.sum
This consolidates on a single place to specify the Go version.
Using the deprecated linter results in the following warning.

    The linter 'exportloopref' is deprecated (since v1.60.2) due to:
    Since Go1.22 (loopvar) this linter is no longer relevant. Replaced
    by copyloopvar."

Offenders of copyloopvar are removed as they result in the following.

    The copy of the 'for' variable "tt" can be deleted (Go 1.22+)
    (copyloopvar)
Vendoring of dependencies is a strategy that can be useful for certain
scenarios. This project does not need to use vendoring and it can be
frustrating to work with as it has subtle side-effects in tooling that
developers need to be aware of. Additionally, when vendoring is used,
vendored dependencies should conventionally be checked in together with
the rest of the code which is currently not done.

Instead of vendoring switch to use go mod download for fetching dependencies
and let the module system take care of the rest.

The go.mod and go.sum file should specify all dependencies to achieve
hermetic builds.
This resolves a few dependency issues that are seemingly causing
friction for security and dependabot updates.

Mainly it drops the replace directives in go.mod and upgrades the
following dependencies.

    k8s.io/cli-runtime
    k8s.io/api
    k8s.io/apimachinery
    k8s.io/kubernetes
    k8s.io/controller-manager
    github.com/argoproj/argo-cd/v2
    github.com/argoproj/gitops-engine@v0.7.1-0.20240905010810-bd7681ae3f8b
    k8s.io/kubectl
    sigs.k8s.io/controller-runtime

One notable difference here is the upgrade of
github.com/argoproj/argo-cd/v2 which further requires a code change to
be compatible with new upstream API: it is now requires to pass in an
installation ID when setting the instance ID for resource tracking.
@hnnsgstfssn hnnsgstfssn merged commit 4406866 into main Jan 21, 2025
5 checks passed
@hnnsgstfssn hnnsgstfssn deleted the maintenance branch January 21, 2025 13:06
@hnnsgstfssn
Copy link
Contributor Author

Making note of two things:

  1. Adding go mod tidy -diff apparently cause all Dependabot pull requests to fail as tidying is not done properly.
  2. It is fine to consolidate on the Go version in go.mod, but just realising that version is meant to indicate the lowest required version and can often be a lot lower to allow older versions to use the module. The project can be compiled with a much newer version of Go, while the code base still supports older versions as it is not making use of newer language features.

For (1) I have opened a support ticket with Github since this should not fail and Dependabot reportedly runs go mod tidy as part of its flow.

For (2) I'd leave things as is for now since I think we're actually using some of the later features. Just noting this.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants