Just migrated our Open Source project to Golang | Hacker News Reader

Just migrated our Open Source project to Golang

github.com

2 pointsujnproduct5 comments

Open articleView on HN

I recommend run a linter against your codebase.

https://golangci-lint.run

https://github.com/go-critic/go-critic

https://github.com/mgechev/revive

And i would actually rename cmd/digger/main.go to cmd/digger/digger.go (_test.go too)

Why? If you go build cmd/digger/main.go you end up with main, if renamed you end up with digger. Without using additional -o flags.

Also consider https://goreleaser.com/intro/

Digger is a Github Action that runs Terraform plan and apply with PR-level locks. The idea is that terraform jobs run natively in your Github Actions - no need to share sensitive data with another CI system. There's no need to deploy and maintain a backend service either. We migrated from Python to Golang yesterday. None of the team had experience with golang, but we managed to migrate in a week. Here's why we did it:

Faster runtimes (upto 30x faster)

Can be compiled into single binary, advantages for Github actions is that we don’t need to wrap action into Dockerfile.

Easy to compile binary to multiple platforms, it helps us run from same codebase.

Interface based development , more guarantees about the code correctness by the compiler when compared to using Python.

Golang is more popular in the DevOps and infrastructure community, we can find several libraries and reuse them in our code.

Would love to hear from HN on our code quality! Please be as critical as possible!

Other code quality issue encountered on a quick review:

  - use of utils package
  - use of models package
  - naming of receivers (too long)
  - returning empty structs instead of (nil) pointers
  - too much log.Fatal scattered around the code
  - unhandled errors, inconsistent error handling
    sometimes handled twice a la log.Printf(..., err) then    return err
    consider error wrapping here
  - useless use of err2 variables
  - use of Must functions which will panic if fail, transition to err producing fns
  - stuttering (digger.DiggerConfig)
  - naming things ...Impl instead of naming the interface ...er || ..or
    This is done correctly in tf.go, not so good in locking.go

For a complete review i would need more time.

While not exactly "code quality," recommending the use of static AWS credentials is an antipattern now that both GitHub and GitLab have working JWT implementations that make `AssumeRoleWithWebIdentity` work correctly, and is supported by all modern aws sdks (including, of course, terraform). One can even configure that exclusively via env-vars so local `terraform plan` works using local credentials, and then the GitHub/GitLab plan uses the ARWWI flow

We actually have an SCP that `Deny`s `iam:CreateAccessKey` in our Organization(s)

Reply on news.ycombinator.com