amazon-ecs-agent icon indicating copy to clipboard operation
amazon-ecs-agent copied to clipboard

Current install-golang.sh script expects go installation in specific directories and doesn't validate go version from GO_VERSIONS file

Open ShelbyZ opened this issue 9 months ago • 2 comments

https://github.com/aws/amazon-ecs-agent/blob/69e95d20fc07bd6bfd922c2e11189b2a5f870435/scripts/install-golang.sh#L40-L66

This approach is a bit brittle when go may be installed via 3rd party tools (such as mise). It may be possible that go is not located under:

  1. /usr/bin/go
  2. /usr/local

In the case of mise I can see that the GOROOT environment variable is set and resolves to a path:

++ which go
/Users/shelbyzh/.local/share/mise/installs/go/1.22.7/bin/go
+++ which go
++ export go_bin_path=/Users/shelbyzh/.local/share/mise/installs/go/1.22.7/bin/go
++ go_bin_path=/Users/shelbyzh/.local/share/mise/installs/go/1.22.7/bin/go
++ '[' /Users/shelbyzh/.local/share/mise/installs/go/1.22.7/bin/go == /usr/bin/go ']'
++ '[' /Users/she == /usr/local ']'
+++ cat ./GO_VERSION
++ echo '/Users/shelbyzh/.local/share/mise/installs/go/1.22.7 doesn'\''t exist, installing 1.22.7'
/Users/shelbyzh/.local/share/mise/installs/go/1.22.7 doesn't exist, installing 1.22.7
+++ cat ./GO_VERSION
++ GO_VERSION=1.22.7
+++ mktemp -d
++ tmpdir=/var/folders/fh/t610t8ld6rl9fhd3r7wnr4540000gq/T/tmp.rsfAhGXuU9
++ GOLANG_TAR=go1.22.7.linux-arm64.tar.gz
++ wget -O /var/folders/fh/t610t8ld6rl9fhd3r7wnr4540000gq/T/tmp.rsfAhGXuU9/go1.22.7.linux-arm64.tar.gz https://storage.googleapis.com/golang/go1.22.7.linux-arm64.tar.gz
--2025-02-26 11:27:22--  https://storage.googleapis.com/golang/go1.22.7.linux-arm64.tar.gz
Resolving storage.googleapis.com (storage.googleapis.com)... 142.251.163.207, 142.251.179.207, 142.251.111.207, ...
Connecting to storage.googleapis.com (storage.googleapis.com)|142.251.163.207|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 65919842 (63M) [application/x-gzip]
Saving to: ‘/var/folders/fh/t610t8ld6rl9fhd3r7wnr4540000gq/T/tmp.rsfAhGXuU9/go1.22.7.linux-arm64.tar.gz’

In the above case it is a a valid go install, but not under expected paths (1/2).

We could change simplify the check as follows:

  1. Check GOROOT environment variable is set
  2. Check go version and validate the output against GO_VERSION file
  3. If 1 or 2 fail, then install the expected GO version

Sample:

# if $GOROOT is not set
if [[ -z $GOROOT ]]; then
  # install go
else
  # validate go version against GO_VERSIONS file
fi

ShelbyZ avatar Feb 26 '25 19:02 ShelbyZ

Hi Shelby 😄

mye956 avatar Mar 14 '25 17:03 mye956

Regarding installing the correct Go version, it looks like it's intentional to not do so in the original PR.

the build targets to prefer either the pre-existing system golang installed at /usr/local/go or else install the GO_VERSION via wget and use that.

To accommodate third-party installations of Golang, we can look to add another condition to rely on whether the GOROOT environment variable is set.

mye956 avatar Mar 14 '25 22:03 mye956