Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

build: bump golangci-lint version in build image #3195

Merged
merged 2 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ linters-settings:
skip-generated: true
stylecheck:
checks: [ "all", "-ST1001", "-ST1005", "-ST1016", "-ST1023", "-ST1000"]
errcheck:
exclude-functions:
- fmt.Fprintln

exclude-dirs:
- pkg/client/applyconfiguration/cr/v1alpha1 # generated from code-gen
Expand Down
2 changes: 1 addition & 1 deletion docker/build/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ COPY --from=goreleaser/goreleaser:v1.26.2 /usr/bin/goreleaser /usr/local/bin/

COPY --from=alpine/helm:3.12.2 /usr/bin/helm /usr/local/bin/

COPY --from=golangci/golangci-lint:v1.57.2 /usr/bin/golangci-lint /usr/local/bin/
COPY --from=golangci/golangci-lint:v1.60.1 /usr/bin/golangci-lint /usr/local/bin/

RUN wget -O /usr/local/bin/kind \
https://github.com/kubernetes-sigs/kind/releases/download/v0.18.0/kind-$(echo $TARGETPLATFORM | tr / -) \
Expand Down
2 changes: 1 addition & 1 deletion pkg/function/create_volume_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func createVolumeSnapshot(ctx context.Context, tp param.TemplateParams, cli kube
}
wg.Wait()

err := fmt.Errorf(strings.Join(errstrings, "\n"))
err := fmt.Errorf("%s", strings.Join(errstrings, "\n"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that this should use errors.New

Suggested change
err := fmt.Errorf("%s", strings.Join(errstrings, "\n"))
err := errors.New(strings.Join(errstrings, "\n"))

Question unrelated to this PR why are newlines (\n) being used inside the error message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe errkit, but I was trying to keep the changes minimal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to errkit because we already have it in this file.

if len(err.Error()) > 0 {
return nil, errkit.Wrap(err, "Failed to snapshot one of the volumes")
}
Expand Down
22 changes: 11 additions & 11 deletions pkg/function/delete_rds_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,22 @@ func (*deleteRDSSnapshotFunc) Name() string {
return DeleteRDSSnapshotFuncName
}

func deleteRDSSnapshot(ctx context.Context, snapshotID string, profile *param.Profile, dbEngine RDSDBEngine) (map[string]interface{}, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes an unparam linter warning.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KastenMike I am assuming this was flagged by a linter as part of the upgrade. @hairyhum can confirm.

If that's the case, there are 2 options:

  • Put all the changes in a single PR, like this one; or
  • 2 PRs in this order: the first one addressing the linter warnings, and the second one upgrading the linter version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upgrading linter version is a github action, which I've already run, so it's just a matter of getting it to the master codebase

func deleteRDSSnapshot(ctx context.Context, snapshotID string, profile *param.Profile, dbEngine RDSDBEngine) error {
// Validate profile
if err := ValidateProfile(profile); err != nil {
return nil, errkit.Wrap(err, "Profile Validation failed")
return errkit.Wrap(err, "Profile Validation failed")
}

// Get aws config from profile
awsConfig, region, err := getAWSConfigFromProfile(ctx, profile)
if err != nil {
return nil, errkit.Wrap(err, "Failed to get AWS creds from profile")
return errkit.Wrap(err, "Failed to get AWS creds from profile")
}

// Create rds client
rdsCli, err := rds.NewClient(ctx, awsConfig, region)
if err != nil {
return nil, errkit.Wrap(err, "Failed to create RDS client")
return errkit.Wrap(err, "Failed to create RDS client")
}

if !isAuroraCluster(string(dbEngine)) {
Expand All @@ -82,16 +82,16 @@ func deleteRDSSnapshot(ctx context.Context, snapshotID string, profile *param.Pr
switch err.Code() {
case awsrds.ErrCodeDBSnapshotNotFoundFault:
log.WithContext(ctx).Print("Could not find matching RDS snapshot; might have been deleted previously", field.M{"SnapshotId": snapshotID})
return nil, nil
return nil
default:
return nil, errkit.Wrap(err, "Failed to delete snapshot")
return errkit.Wrap(err, "Failed to delete snapshot")
}
}
}
// Wait until snapshot is deleted
log.WithContext(ctx).Print("Waiting for RDS snapshot to be deleted", field.M{"SnapshotID": snapshotID})
err = rdsCli.WaitUntilDBSnapshotDeleted(ctx, snapshotID)
return nil, errkit.Wrap(err, "Error while waiting for snapshot to be deleted")
return errkit.Wrap(err, "Error while waiting for snapshot to be deleted")
}

// delete Aurora DB cluster snapshot
Expand All @@ -102,17 +102,17 @@ func deleteRDSSnapshot(ctx context.Context, snapshotID string, profile *param.Pr
switch err.Code() {
case awsrds.ErrCodeDBClusterSnapshotNotFoundFault:
log.WithContext(ctx).Print("Could not find matching Aurora DB cluster snapshot; might have been deleted previously", field.M{"SnapshotId": snapshotID})
return nil, nil
return nil
default:
return nil, errkit.Wrap(err, "Error deleting Aurora DB cluster snapshot")
return errkit.Wrap(err, "Error deleting Aurora DB cluster snapshot")
}
}
}

log.WithContext(ctx).Print("Waiting for Aurora DB cluster snapshot to be deleted")
err = rdsCli.WaitUntilDBClusterDeleted(ctx, snapshotID)

return nil, errkit.Wrap(err, "Error waiting for Aurora DB cluster snapshot to be deleted")
return errkit.Wrap(err, "Error waiting for Aurora DB cluster snapshot to be deleted")
}

func (d *deleteRDSSnapshotFunc) Exec(ctx context.Context, tp param.TemplateParams, args map[string]interface{}) (map[string]interface{}, error) {
Expand All @@ -130,7 +130,7 @@ func (d *deleteRDSSnapshotFunc) Exec(ctx context.Context, tp param.TemplateParam
return nil, err
}

return deleteRDSSnapshot(ctx, snapshotID, tp.Profile, dbEngine)
return nil, deleteRDSSnapshot(ctx, snapshotID, tp.Profile, dbEngine)
}

func (*deleteRDSSnapshotFunc) RequiredArgs() []string {
Expand Down
2 changes: 1 addition & 1 deletion pkg/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func (*healthCheckHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}
w.WriteHeader(http.StatusOK)
w.Header().Set("Content-Type", "application/json")
_, _ = io.WriteString(w, string(js))
_, _ = io.Writer.Write(w, js)
}

// RunWebhookServer starts the validating webhook resources for blueprint kanister resources
Expand Down
2 changes: 1 addition & 1 deletion pkg/kube/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func GetPodObjectFromPodOptions(ctx context.Context, cli kubernetes.Interface, o
ServiceAccountName: sa,
}

if opts.EnvironmentVariables != nil && len(opts.EnvironmentVariables) > 0 {
if len(opts.EnvironmentVariables) > 0 {
defaultSpecs.Containers[0].Env = opts.EnvironmentVariables
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kube/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func (s *PodSuite) TestPod(c *check.C) {
c.Assert(pod.Spec.RestartPolicy, check.Equals, po.RestartPolicy)
}

if po.EnvironmentVariables != nil && len(po.EnvironmentVariables) > 0 {
if len(po.EnvironmentVariables) > 0 {
c.Assert(pod.Spec.Containers[0].Env, check.DeepEquals, po.EnvironmentVariables)
}

Expand Down