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

all: Bump Golang minor versions in accordance with latest release. #3693

Merged
merged 5 commits into from
Jun 16, 2021

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Jun 15, 2021

What

Bumps from 1.16.3 -> 1.16.5 and 1.15.11 -> 1.15.13.

Why

This bump includes security issues. Refer to the release announcement for more.

@Shaptic Shaptic requested a review from a team June 15, 2021 18:20
@Shaptic Shaptic self-assigned this Jun 15, 2021
@Shaptic
Copy link
Contributor Author

Shaptic commented Jun 15, 2021

I found these references based on Ctrl+F on things like go:, 1.16, golang:, and 1.15, therefore it may not be all of the possible references, but it damn-sure is a lot of them.

@@ -227,7 +227,7 @@ jobs:
check_code_1_16:
working_directory: /go/src/github.com/stellar/go
docker:
- image: golang:1.16
- image: golang:1.16.5
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should replace it with:

Suggested change
- image: golang:1.16.5
- image: golang:1.16

so that it will always pull latest Golang 1.16.x. This was recommended by @jacekn and @satyamz in the issue about bumping Golang versions in SDF. Same thing for 1.15.x.

Copy link
Member

Choose a reason for hiding this comment

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

I like this idea. We basically always want to be on latest patch release. Let's do it.

For minor versions let's stick to manual update for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I didn't realize 1.16 would always be the latest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@Shaptic Shaptic requested a review from bartekn June 16, 2021 17:42
services/horizon/docker/Dockerfile.dev Outdated Show resolved Hide resolved
.github/workflows/go.yml Show resolved Hide resolved
Co-authored-by: Bartek Nowotarski <[email protected]>
@Shaptic Shaptic merged commit aaec655 into stellar:master Jun 16, 2021
@Shaptic Shaptic deleted the bump-golang branch June 16, 2021 23:22
@@ -1,4 +1,4 @@
FROM golang:1.16.3 as build
Copy link
Member

Choose a reason for hiding this comment

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

Why did we remove the patch version from some locations like this one, but we kept the patch versions in other services?

Copy link
Member

Choose a reason for hiding this comment

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

are you talking about these? #3693 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

That link doesn't seem to go anywhere for me. In the CI files patch versions are used, and in the Ticker, Keystore, and other docker files a patch version is used. It'd be helpful if we are consistent.

Copy link
Contributor Author

@Shaptic Shaptic Jun 22, 2021

Choose a reason for hiding this comment

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

Using the patch version means updating the patch version.
Using the minor version for Docker images means we always get the latest patch version (see above).
We can't do this for things like download URLs (see above), however, so we still need the patch version sometimes.

Copy link
Member

Choose a reason for hiding this comment

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

Got it. What's the motivation for only making that change for Friendbot and Horizon, and not the other services?

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

Successfully merging this pull request may close these issues.

None yet

5 participants