-
Notifications
You must be signed in to change notification settings - Fork 101
feat: add ops/gmpctl interactive scripts #1817
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
base: main
Are you sure you want to change the base?
Conversation
See go/gmp:toil-automation for motivation. * listing Go vulns + severity * create a security vuln fix commit * create a new fork release that syncs with certain upstream tag (see go/gmp:fork-toil) * cut RC Signed-off-by: bwplotka <bwplotka@google.com> chore: update chore: add go version check Signed-off-by: bwplotka <bwplotka@gmail.com>
77d1159 to
ee97a7d
Compare
Signed-off-by: bwplotka <bwplotka@gmail.com>
73f3c0f to
c264e29
Compare
d6d8e27 to
7a07b02
Compare
hsmatulis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just a plumbing issue
562deb1 to
0748cd6
Compare
| // ReleaseBranches contains hardcoded list of active branches. We could pull it out from somewhere. | ||
| ReleaseBranches = []string{ | ||
| "release/0.17", | ||
| "release/0.15", | ||
| "release/0.14", | ||
| "release/0.12", | ||
| "release-2.45.3-gmp", | ||
| "release-2.53.5-gmp", | ||
| "release-0.27.0-gmp", | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not hardcode these. Otherwise, this becomes another point of manual maintenance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you suggest instead? Putting them in a static YAML and committed is similarly "hardcoded". Having this locally for each of us will also get obsolete even quicker.
It's hard to pull it from g3 on demand.
I can add TODO, but I don't see a quick path for not hardcoding those for. What am I missing?
Perhaps we can pull some of it from github actions dependabot for PE branches? Or maintain some other coordinated config?
| // compileUpdateList decodes the JSON stream from govulncheck and extracts | ||
| // a list of modules that need to be updated to a fixed version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need Go code to accomplish this? Can we just run an existing tool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What "just run an existing tool" means for you here?
If you mean why another Go command, I agree, ideally I move vulnupdatelist as another gmpctl command or just add as a library, will add TODO or move 👍🏽
Or did you mean anything else?
| SHFMT := $(GOBIN)/shfmt-v3.12.0 | ||
| $(SHFMT): $(BINGO_DIR)/shfmt.mod | ||
| @# Install binary/ries using Go 1.14+ build command. This is using bwplotka/bingo-controlled, separate go module with pinned dependencies. | ||
| @echo "(re)installing $(GOBIN)/shfmt-v3.12.0" | ||
| @cd $(BINGO_DIR) && GOWORK=off GOOS=$(GOHOSTOS) GOARCH=$(GOHOSTARCH) GOARM=$(GOHOSTARM) $(GO) build -mod=mod -modfile=shfmt.mod -o=$(GOBIN)/shfmt-v3.12.0 "mvdan.cc/sh/v3/cmd/shfmt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we still using Bingo? Most tools have moved to go tool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, looks like we do because we have this file. 🙈
Do you ask me to move from bingo to go tool here in this PR, now? (:
| // runLibFunction runs certain function from libScript that is not expected | ||
| // to pass any return parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have to jump back and forth between Go code and Bash scripts? Why not implement these bash functions in Go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of time constraints, because of how quickly I could experiment with bash. So it is an iteration. I'm sorry but I won't be able to port everything in this iteration, I will try to do some.
I would love to prioritize use and enablement of automation for our release process for the team.
Is any bash use a blocker for you?
bernot-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I count 33 TODOs. That seems like a lot of TODO debt to take on for one PR.
Well, it's either that or manual process 10 steps on every vulnfix/release. Which one do we prefer? 🙈 Thanks for feedback! It's an iteration and I do need team help to make it better and portable for everyone. Let's iterate. BTW: Anyone is welcome to send PRs to this PR etc to contribute the improvements 🤗 |
See
ops/gmpctl/README.mdand cl/840052660 for details.I removed release bot given
ops/gmpctl.sh releaseis now trivial, @hsmatulis agreed.