diff --git a/.golangci.yml b/.golangci.yml index 81d4db90936d0..ec268e7080baf 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -122,7 +122,9 @@ linters-settings: - pkg: golang.org/x/exp/slices desc: 'use "slices" instead' - pkg: github.com/hashicorp/go-version - desc: 'use "golang.org/x/mod/semver" or "coreos/go-semver/semver" instead' + desc: 'use "coreos/go-semver/semver" instead' + - pkg: golang.org/x/mod/semver + desc: 'use "coreos/go-semver/semver" instead' - pkg: github.com/microsoftgraph/msgraph-sdk-go desc: 'use "github.com/gravitational/teleport/lib/msgraph" instead' - pkg: github.com/cloudflare/cfssl diff --git a/build.assets/tooling/cmd/check/main.go b/build.assets/tooling/cmd/check/main.go index 75cdde7fb7eb3..336c5b07003c6 100644 --- a/build.assets/tooling/cmd/check/main.go +++ b/build.assets/tooling/cmd/check/main.go @@ -30,7 +30,7 @@ import ( "time" "github.com/gravitational/trace" - "golang.org/x/mod/semver" + "golang.org/x/mod/semver" //nolint:depguard // Usage precedes the x/mod/semver rule. "github.com/gravitational/teleport/build.assets/tooling/lib/github" ) diff --git a/build.assets/tooling/cmd/query-latest/main.go b/build.assets/tooling/cmd/query-latest/main.go index fae3d9076cdc4..4f565e239eadb 100644 --- a/build.assets/tooling/cmd/query-latest/main.go +++ b/build.assets/tooling/cmd/query-latest/main.go @@ -37,7 +37,7 @@ import ( "time" "github.com/gravitational/trace" - "golang.org/x/mod/semver" + "golang.org/x/mod/semver" //nolint:depguard // Usage precedes the x/mod/semver rule. "github.com/gravitational/teleport/build.assets/tooling/lib/github" ) diff --git a/e_imports.go b/e_imports.go index 7a2fd5cf466ea..cdeed89fb8b19 100644 --- a/e_imports.go +++ b/e_imports.go @@ -119,7 +119,7 @@ import ( _ "golang.org/x/crypto/ssh" _ "golang.org/x/crypto/ssh/agent" _ "golang.org/x/exp/constraints" - _ "golang.org/x/mod/semver" + _ "golang.org/x/mod/semver" //nolint:depguard // Usage precedes the x/mod/semver rule. _ "golang.org/x/net/html" _ "golang.org/x/net/http/httpproxy" _ "golang.org/x/net/http2" diff --git a/integrations/kube-agent-updater/cmd/teleport-kube-agent-updater/main.go b/integrations/kube-agent-updater/cmd/teleport-kube-agent-updater/main.go index aafba67be0414..3ffe2072a4ead 100644 --- a/integrations/kube-agent-updater/cmd/teleport-kube-agent-updater/main.go +++ b/integrations/kube-agent-updater/cmd/teleport-kube-agent-updater/main.go @@ -30,7 +30,6 @@ import ( "github.com/distribution/reference" "github.com/go-logr/logr" "github.com/gravitational/trace" - "golang.org/x/mod/semver" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/fields" @@ -252,7 +251,7 @@ func main() { case insecureNoVerify: ctrl.Log.Info("INSECURE: Image validation disabled") imageValidators = append(imageValidators, img.NewInsecureValidator("insecure always verified", kc)) - case semver.Prerelease("v"+kubeversionupdater.Version) != "": + case kubeversionupdater.SemVersion != nil && kubeversionupdater.SemVersion.PreRelease != "": ctrl.Log.Info("This is a pre-release updater version, the key used to sign dev and pre-release builds of Teleport will be trusted.") validator, err := img.NewCosignSingleKeyValidator(teleportStageOCIPubKey, "staging cosign signature validator", kc) if err != nil { diff --git a/integrations/kube-agent-updater/pkg/controller/updater.go b/integrations/kube-agent-updater/pkg/controller/updater.go index a55df120caa6c..197d6a52109ee 100644 --- a/integrations/kube-agent-updater/pkg/controller/updater.go +++ b/integrations/kube-agent-updater/pkg/controller/updater.go @@ -20,8 +20,8 @@ package controller import ( "context" - "strings" + "github.com/coreos/go-semver/semver" "github.com/distribution/reference" "github.com/gravitational/trace" "sigs.k8s.io/controller-runtime/pkg/client" @@ -44,7 +44,7 @@ type VersionUpdater struct { // validating the new image signature. // If all steps are successfully executed and there's a new version, it returns // a digested reference to the new image that should be deployed. -func (r *VersionUpdater) GetVersion(ctx context.Context, obj client.Object, currentVersion string) (img.NamedTaggedDigested, error) { +func (r *VersionUpdater) GetVersion(ctx context.Context, obj client.Object, currentVersion *semver.Version) (img.NamedTaggedDigested, error) { // Those are debug logs only log := ctrllog.FromContext(ctx).V(1) @@ -68,7 +68,7 @@ func (r *VersionUpdater) GetVersion(ctx context.Context, obj client.Object, curr log.Info("Version change is valid, building img candidate") // We tag our img candidate with the version - image, err := reference.WithTag(r.baseImage, strings.TrimPrefix(nextVersion, "v")) + image, err := reference.WithTag(r.baseImage, nextVersion.String()) if err != nil { return nil, trace.Wrap(err) } diff --git a/integrations/kube-agent-updater/pkg/controller/updater_test.go b/integrations/kube-agent-updater/pkg/controller/updater_test.go index c97fb9d94c917..be638cdf6cbc7 100644 --- a/integrations/kube-agent-updater/pkg/controller/updater_test.go +++ b/integrations/kube-agent-updater/pkg/controller/updater_test.go @@ -23,6 +23,7 @@ import ( "fmt" "testing" + "github.com/coreos/go-semver/semver" "github.com/distribution/reference" "github.com/gravitational/trace" "github.com/opencontainers/go-digest" @@ -63,6 +64,13 @@ func errorIsType(errType interface{}) require.ErrorAssertionFunc { } } +func mustNewStaticGetter(t *testing.T, versionMock string, errMock error) version.Getter { + t.Helper() + getter, err := version.NewStaticGetter(versionMock, errMock) + require.NoError(t, err) + return getter +} + func Test_VersionUpdater_GetVersion(t *testing.T) { ctx := context.Background() @@ -70,7 +78,7 @@ func Test_VersionUpdater_GetVersion(t *testing.T) { name string releaseRegistry string releasePath string - currentVersion string + currentVersion *semver.Version versionGetter version.Getter maintenanceTriggers []maintenance.Trigger imageCheckers []img.Validator @@ -81,8 +89,8 @@ func Test_VersionUpdater_GetVersion(t *testing.T) { name: "all good", releaseRegistry: defaultTestRegistry, releasePath: defaultTestPath, - currentVersion: versionMid, - versionGetter: version.NewStaticGetter(versionHigh, nil), + currentVersion: semver.Must(version.EnsureSemver(versionMid)), + versionGetter: mustNewStaticGetter(t, versionHigh, nil), maintenanceTriggers: []maintenance.Trigger{alwaysTrigger}, imageCheckers: []img.Validator{alwaysValid}, assertErr: require.NoError, @@ -92,8 +100,8 @@ func Test_VersionUpdater_GetVersion(t *testing.T) { name: "all good but no current version", releaseRegistry: defaultTestRegistry, releasePath: defaultTestPath, - currentVersion: "", - versionGetter: version.NewStaticGetter(versionHigh, nil), + currentVersion: nil, + versionGetter: mustNewStaticGetter(t, versionHigh, nil), maintenanceTriggers: []maintenance.Trigger{alwaysTrigger}, imageCheckers: []img.Validator{alwaysValid}, assertErr: require.NoError, @@ -103,8 +111,8 @@ func Test_VersionUpdater_GetVersion(t *testing.T) { name: "same version", releaseRegistry: defaultTestRegistry, releasePath: defaultTestPath, - currentVersion: versionMid, - versionGetter: version.NewStaticGetter(versionMid, nil), + currentVersion: semver.Must(version.EnsureSemver(versionMid)), + versionGetter: mustNewStaticGetter(t, versionMid, nil), maintenanceTriggers: []maintenance.Trigger{alwaysTrigger}, imageCheckers: []img.Validator{alwaysValid}, assertErr: errorIsType(&version.NoNewVersionError{}), @@ -114,8 +122,8 @@ func Test_VersionUpdater_GetVersion(t *testing.T) { name: "no version", releaseRegistry: defaultTestRegistry, releasePath: defaultTestPath, - currentVersion: versionMid, - versionGetter: version.NewStaticGetter("", &version.NoNewVersionError{Message: "version server did not advertise a version"}), + currentVersion: semver.Must(version.EnsureSemver(versionMid)), + versionGetter: mustNewStaticGetter(t, "", &version.NoNewVersionError{Message: "version server did not advertise a version"}), maintenanceTriggers: []maintenance.Trigger{alwaysTrigger}, imageCheckers: []img.Validator{alwaysValid}, assertErr: errorIsType(&version.NoNewVersionError{}), @@ -125,8 +133,8 @@ func Test_VersionUpdater_GetVersion(t *testing.T) { name: "no maintenance triggered", releaseRegistry: defaultTestRegistry, releasePath: defaultTestPath, - currentVersion: versionMid, - versionGetter: version.NewStaticGetter(versionHigh, nil), + currentVersion: semver.Must(version.EnsureSemver(versionMid)), + versionGetter: mustNewStaticGetter(t, versionHigh, nil), maintenanceTriggers: []maintenance.Trigger{neverTrigger}, imageCheckers: []img.Validator{alwaysValid}, assertErr: errorIsType(&MaintenanceNotTriggeredError{}), @@ -136,8 +144,8 @@ func Test_VersionUpdater_GetVersion(t *testing.T) { name: "invalid signature", releaseRegistry: defaultTestRegistry, releasePath: defaultTestPath, - currentVersion: versionMid, - versionGetter: version.NewStaticGetter(versionHigh, nil), + currentVersion: semver.Must(version.EnsureSemver(versionMid)), + versionGetter: mustNewStaticGetter(t, versionHigh, nil), maintenanceTriggers: []maintenance.Trigger{alwaysTrigger}, imageCheckers: []img.Validator{neverValid}, assertErr: errorIsType(&trace.TrustError{}), @@ -147,8 +155,8 @@ func Test_VersionUpdater_GetVersion(t *testing.T) { name: "error getting version", releaseRegistry: defaultTestRegistry, releasePath: defaultTestPath, - currentVersion: versionMid, - versionGetter: version.NewStaticGetter("", &trace.ConnectionProblemError{}), + currentVersion: semver.Must(version.EnsureSemver(versionMid)), + versionGetter: mustNewStaticGetter(t, "", &trace.ConnectionProblemError{}), maintenanceTriggers: []maintenance.Trigger{alwaysTrigger}, imageCheckers: []img.Validator{neverValid}, assertErr: errorIsType(&trace.ConnectionProblemError{}), @@ -176,7 +184,7 @@ func Test_VersionUpdater_GetVersion(t *testing.T) { obj := &core.Pod{} // Doing the test - image, err := updater.GetVersion(ctx, obj, "v"+tt.currentVersion) + image, err := updater.GetVersion(ctx, obj, tt.currentVersion) tt.assertErr(t, err) if tt.expectedImage == "" { require.Nil(t, image) diff --git a/integrations/kube-agent-updater/pkg/controller/utils.go b/integrations/kube-agent-updater/pkg/controller/utils.go index 4625b67213ef2..5fe4f53b63bf9 100644 --- a/integrations/kube-agent-updater/pkg/controller/utils.go +++ b/integrations/kube-agent-updater/pkg/controller/utils.go @@ -21,6 +21,7 @@ package controller import ( "strconv" + "github.com/coreos/go-semver/semver" "github.com/distribution/reference" "github.com/gravitational/trace" v1 "k8s.io/api/core/v1" @@ -29,27 +30,26 @@ import ( "github.com/gravitational/teleport/lib/automaticupgrades/version" ) -func getWorkloadVersion(podSpec v1.PodSpec) (string, error) { - var current string +func getWorkloadVersion(podSpec v1.PodSpec) (*semver.Version, error) { image, err := getContainerImageFromPodSpec(podSpec, teleportContainerName) if err != nil { - return current, trace.Wrap(err) + return nil, trace.Wrap(err) } imageRef, err := reference.ParseNamed(image) if err != nil { - return current, trace.Wrap(err) + return nil, trace.Wrap(err) } taggedImageRef, ok := imageRef.(reference.Tagged) if !ok { - return "", trace.BadParameter("imageRef %s is not tagged", imageRef) + return nil, trace.BadParameter("imageRef %s is not tagged", imageRef) } - current = taggedImageRef.Tag() - current, err = version.EnsureSemver(current) + currentTag := taggedImageRef.Tag() + currentVersion, err := version.EnsureSemver(currentTag) if err != nil { - return "", trace.Wrap(err) + return nil, trace.BadParameter("failed to parse image version: %v", err) } - return current, nil + return currentVersion, nil } func getContainerImageFromPodSpec(spec v1.PodSpec, container string) (string, error) { diff --git a/integrations/kube-agent-updater/pkg/controller/utils_test.go b/integrations/kube-agent-updater/pkg/controller/utils_test.go index 1432f6f75b1bf..88d40c2e8296d 100644 --- a/integrations/kube-agent-updater/pkg/controller/utils_test.go +++ b/integrations/kube-agent-updater/pkg/controller/utils_test.go @@ -21,9 +21,12 @@ package controller import ( "testing" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" + + "github.com/gravitational/teleport/lib/automaticupgrades/version" ) const ( @@ -266,46 +269,49 @@ func newPodSpecWithImage(image string) v1.PodSpec { } func Test_getWorkloadVersion(t *testing.T) { + testVersion, err := version.EnsureSemver(versionMid) + require.NoError(t, err) + tests := []struct { name string podSpec v1.PodSpec - expected string + expected *semver.Version assertErr require.ErrorAssertionFunc }{ { name: "OK regular podSpec, semver tag no digest", podSpec: newPodSpecWithImage(defaultTestRegistry + "/" + defaultTestPath + ":" + versionMid), - expected: "v" + versionMid, + expected: testVersion, assertErr: require.NoError, }, { name: "OK regular podSpec, semver tag with digest", podSpec: newPodSpecWithImage(defaultTestRegistry + "/" + defaultTestPath + ":" + versionMid + "@" + defaultImageDigest.String()), - expected: "v" + versionMid, + expected: testVersion, assertErr: require.NoError, }, { name: "KO regular podSpec, non-semver tag no digest", podSpec: newPodSpecWithImage(defaultTestRegistry + "/" + defaultTestPath + ":" + nonSemverTag), - expected: "", + expected: nil, assertErr: errorIsType(&trace.BadParameterError{}), }, { name: "KO regular podSpec, non-semver tag with digest", podSpec: newPodSpecWithImage(defaultTestRegistry + "/" + defaultTestPath + ":" + nonSemverTag + "@" + defaultImageDigest.String()), - expected: "", + expected: nil, assertErr: errorIsType(&trace.BadParameterError{}), }, { name: "KO regular podSpec, no tag, only digest", podSpec: newPodSpecWithImage(defaultTestRegistry + "/" + defaultTestPath + "@" + defaultImageDigest.String()), - expected: "", + expected: nil, assertErr: errorIsType(&trace.BadParameterError{}), }, { name: "KO regular podSpec, no tag, no digest", podSpec: newPodSpecWithImage(defaultTestRegistry + "/" + defaultTestPath), - expected: "", + expected: nil, assertErr: errorIsType(&trace.BadParameterError{}), }, { @@ -322,7 +328,7 @@ func Test_getWorkloadVersion(t *testing.T) { }, }, }, - expected: "v" + versionMid, + expected: testVersion, assertErr: require.NoError, }, { @@ -335,7 +341,7 @@ func Test_getWorkloadVersion(t *testing.T) { }, }, }, - expected: "", + expected: nil, assertErr: require.Error, }, } diff --git a/integrations/kube-agent-updater/version.go b/integrations/kube-agent-updater/version.go index 4b5352f9c6c3a..e0801dfc1a2cb 100644 --- a/integrations/kube-agent-updater/version.go +++ b/integrations/kube-agent-updater/version.go @@ -21,3 +21,5 @@ package kubeversionupdater import "github.com/gravitational/teleport/api" const Version = api.Version + +var SemVersion = api.SemVersion diff --git a/lib/auth/periodic.go b/lib/auth/periodic.go index ce36934f02d01..6ed328916360d 100644 --- a/lib/auth/periodic.go +++ b/lib/auth/periodic.go @@ -23,7 +23,7 @@ import ( "slices" "strings" - "golang.org/x/mod/semver" + "golang.org/x/mod/semver" //nolint:depguard // Usage precedes the x/mod/semver rule. "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/types" diff --git a/lib/automaticupgrades/channel.go b/lib/automaticupgrades/channel.go index 9260bd675a91a..f90ea648ecb12 100644 --- a/lib/automaticupgrades/channel.go +++ b/lib/automaticupgrades/channel.go @@ -22,12 +22,10 @@ import ( "context" "log/slog" "net/url" - "strconv" - "strings" "sync" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" - "golang.org/x/mod/semver" "github.com/gravitational/teleport" "github.com/gravitational/teleport/lib/automaticupgrades/maintenance" @@ -90,10 +88,10 @@ func (c Channels) CheckAndSetDefaults() error { } // DefaultVersion returns the version served by the default upgrade channel. -func (c Channels) DefaultVersion(ctx context.Context) (string, error) { +func (c Channels) DefaultVersion(ctx context.Context) (*semver.Version, error) { channel, ok := c[DefaultChannelName] if !ok { - return "", trace.NotFound("default version channel not found") + return nil, trace.NotFound("default version channel not found") } targetVersion, err := channel.GetVersion(ctx) return targetVersion, trace.Wrap(err) @@ -125,7 +123,7 @@ type Channel struct { criticalTrigger maintenance.Trigger // teleportMajor stores the current teleport major for comparison. // This field is initialized during CheckAndSetDefaults. - teleportMajor int + teleportMajor int64 // mutex protects versionGetter, criticalTrigger, and teleportMajor mutex sync.Mutex } @@ -147,17 +145,17 @@ func (c *Channel) CheckAndSetDefaults() error { c.versionGetter = version.NewBasicHTTPVersionGetter(baseURL) c.criticalTrigger = maintenance.NewBasicHTTPMaintenanceTrigger("remote", baseURL) case c.StaticVersion != "": - c.versionGetter = version.NewStaticGetter(c.StaticVersion, nil) + var err error + c.versionGetter, err = version.NewStaticGetter(c.StaticVersion, nil) + if err != nil { + return trace.Wrap(err) + } c.criticalTrigger = maintenance.NewMaintenanceStaticTrigger("remote", c.Critical) default: return trace.BadParameter("either ForwardURL or StaticVersion must be set") } - var err error - c.teleportMajor, err = parseMajorFromVersionString(teleport.Version) - if err != nil { - return trace.Wrap(err, "failed to process teleport version") - } + c.teleportMajor = teleport.SemVersion.Major return nil } @@ -169,25 +167,20 @@ func (c *Channel) CheckAndSetDefaults() error { // returns the Teleport version instead. // If the version source intentionally did not specify a version, a // NoNewVersionError is returned. -func (c *Channel) GetVersion(ctx context.Context) (string, error) { +func (c *Channel) GetVersion(ctx context.Context) (*semver.Version, error) { c.mutex.Lock() defer c.mutex.Unlock() targetVersion, err := c.versionGetter.GetVersion(ctx) if err != nil { - return "", trace.Wrap(err) - } - - targetMajor, err := parseMajorFromVersionString(targetVersion) - if err != nil { - return "", trace.Wrap(err, "failed to process target version") + return nil, trace.Wrap(err) } // The target version is officially incompatible with our version, // we prefer returning our version rather than having a broken client - if targetMajor > c.teleportMajor { + if targetVersion.Major > c.teleportMajor { targetVersion, err = version.EnsureSemver(teleport.Version) if err != nil { - return "", trace.Wrap(err, "ensuring current teleport version is semver-compatible") + return nil, trace.Wrap(err, "ensuring current teleport version is semver-compatible") } } @@ -233,17 +226,3 @@ var newDefaultChannel = sync.OnceValues[*Channel, error]( func NewDefaultChannel() (*Channel, error) { return newDefaultChannel() } - -func parseMajorFromVersionString(v string) (int, error) { - v, err := version.EnsureSemver(v) - if err != nil { - return 0, trace.Wrap(err, "invalid semver: %s", v) - } - majorStr := semver.Major(v) - if majorStr == "" { - return 0, trace.BadParameter("cannot detect version major") - } - - major, err := strconv.Atoi(strings.TrimPrefix(majorStr, "v")) - return major, trace.Wrap(err, "cannot convert version major to int") -} diff --git a/lib/automaticupgrades/channel_test.go b/lib/automaticupgrades/channel_test.go index 484b82bce4f97..94c4eed1d3baf 100644 --- a/lib/automaticupgrades/channel_test.go +++ b/lib/automaticupgrades/channel_test.go @@ -22,6 +22,7 @@ import ( "context" "testing" + "github.com/coreos/go-semver/semver" "github.com/stretchr/testify/require" "github.com/gravitational/teleport" @@ -210,41 +211,46 @@ func Test_Channel_GetVersion(t *testing.T) { tests := []struct { name string targetVersion string - expectedVersion string + expectedVersion *semver.Version assertErr require.ErrorAssertionFunc + assertCheckErr require.ErrorAssertionFunc }{ { name: "normal version", targetVersion: "v1.2.3", - expectedVersion: "v1.2.3", + expectedVersion: semver.Must(version.EnsureSemver("v1.2.3")), assertErr: require.NoError, }, { name: "no version", targetVersion: constants.NoVersion, - expectedVersion: "", + expectedVersion: nil, assertErr: require.Error, }, { name: "version too high", targetVersion: "v99.1.1", - expectedVersion: "v" + teleport.Version, + expectedVersion: teleport.SemVersion, assertErr: require.NoError, }, { - name: "version invalid", - targetVersion: "foobar", - assertErr: require.Error, + name: "version invalid", + targetVersion: "foobar", + assertCheckErr: require.Error, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ch := Channel{StaticVersion: tt.targetVersion} - require.NoError(t, ch.CheckAndSetDefaults()) + if tt.assertCheckErr != nil { + tt.assertCheckErr(t, ch.CheckAndSetDefaults()) + } else { + require.NoError(t, ch.CheckAndSetDefaults()) + result, err := ch.GetVersion(ctx) - result, err := ch.GetVersion(ctx) - tt.assertErr(t, err) - require.Equal(t, tt.expectedVersion, result) + tt.assertErr(t, err) + require.Equal(t, tt.expectedVersion, result) + } }) } } diff --git a/lib/automaticupgrades/config.go b/lib/automaticupgrades/config.go index 67d45648d5abe..01d91f6a8fcf5 100644 --- a/lib/automaticupgrades/config.go +++ b/lib/automaticupgrades/config.go @@ -19,13 +19,14 @@ package automaticupgrades import ( - "bytes" "context" "log/slog" "os" "os/exec" "strconv" + "github.com/coreos/go-semver/semver" + "github.com/gravitational/teleport/lib/automaticupgrades/version" ) @@ -74,19 +75,32 @@ func GetChannel() string { } // GetUpgraderVersion returns the teleport upgrader version -func GetUpgraderVersion(ctx context.Context) string { +func GetUpgraderVersion(ctx context.Context) *semver.Version { if os.Getenv(EnvUpgrader) == "unit" { out, err := exec.CommandContext(ctx, teleportUpgradeScript, "version").Output() if err != nil { slog.DebugContext(ctx, "Failed to exec /usr/sbin/teleport-upgrade version command", "error", err) - return "" + return nil } - ver, err := version.EnsureSemver(string(bytes.TrimSpace(out))) + ver, err := version.EnsureSemver(string(out)) if err != nil { slog.DebugContext(ctx, "Unexpected teleport-upgrade version", "error", err) - return "" + return nil } return ver } - return os.Getenv(EnvUpgraderVersion) + env := os.Getenv(EnvUpgraderVersion) + if env == "" { + return nil + } + + ver, err := version.EnsureSemver(env) + if err != nil { + slog.WarnContext(ctx, "Unexpected updater version in env var", + "error", err, + "env_name", os.Getenv(EnvUpgraderVersion), + "env_value", env) + return nil + } + return ver } diff --git a/lib/automaticupgrades/version/basichttp.go b/lib/automaticupgrades/version/basichttp.go index 29bea8e325861..ee4a5cefae9ac 100644 --- a/lib/automaticupgrades/version/basichttp.go +++ b/lib/automaticupgrades/version/basichttp.go @@ -22,8 +22,8 @@ import ( "context" "net/http" "net/url" - "strings" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/gravitational/teleport/lib/automaticupgrades/basichttp" @@ -43,18 +43,17 @@ type basicHTTPVersionClient struct { // Get sends an HTTP GET request and returns the version prefixed by "v". // It expects the endpoint to be unauthenticated, return 200 and the response // body to contain a valid semver tag without the "v". -func (b *basicHTTPVersionClient) Get(ctx context.Context) (string, error) { +func (b *basicHTTPVersionClient) Get(ctx context.Context) (*semver.Version, error) { versionURL := b.baseURL.JoinPath(constants.VersionPath) body, err := b.client.GetContent(ctx, *versionURL) if err != nil { - return "", trace.Wrap(err, "failed to get version from %s", versionURL) + return nil, trace.Wrap(err, "failed to get version from %s", versionURL) } response := string(body) if response == constants.NoVersion { - return "", &NoNewVersionError{Message: "version server did not advertise a version"} + return nil, &NoNewVersionError{Message: "version server did not advertise a version"} } - // We trim spaces because the value might end with one or many newlines - version, err := EnsureSemver(strings.TrimSpace(response)) + version, err := EnsureSemver(response) return version, trace.Wrap(err) } @@ -65,10 +64,10 @@ func (b *basicHTTPVersionClient) Get(ctx context.Context) (string, error) { // in order to mitigate the impact of too frequent reconciliations. // The structure implements the version.Getter interface. type BasicHTTPVersionGetter struct { - versionGetter func(context.Context) (string, error) + versionGetter func(context.Context) (*semver.Version, error) } -func (g BasicHTTPVersionGetter) GetVersion(ctx context.Context) (string, error) { +func (g BasicHTTPVersionGetter) GetVersion(ctx context.Context) (*semver.Version, error) { return g.versionGetter(ctx) } @@ -81,5 +80,5 @@ func NewBasicHTTPVersionGetter(baseURL *url.URL) Getter { client: &basichttp.Client{Client: client}, } - return BasicHTTPVersionGetter{cache.NewTimedMemoize[string](httpVersionClient.Get, constants.CacheDuration).Get} + return BasicHTTPVersionGetter{cache.NewTimedMemoize[*semver.Version](httpVersionClient.Get, constants.CacheDuration).Get} } diff --git a/lib/automaticupgrades/version/basichttp_test.go b/lib/automaticupgrades/version/basichttp_test.go index 8dd82d95352fa..ac77624a95867 100644 --- a/lib/automaticupgrades/version/basichttp_test.go +++ b/lib/automaticupgrades/version/basichttp_test.go @@ -24,6 +24,7 @@ import ( "net/url" "testing" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/stretchr/testify/require" @@ -45,28 +46,35 @@ func Test_basicHTTPVersionClient_Get(t *testing.T) { name string statusCode int response string - expected string + expected *semver.Version assertErr require.ErrorAssertionFunc }{ { name: "all good", statusCode: http.StatusOK, response: "12.0.3", - expected: "v12.0.3", + expected: semver.Must(EnsureSemver("12.0.3")), assertErr: require.NoError, }, { name: "all good with newline", statusCode: http.StatusOK, response: "12.0.3\n", - expected: "v12.0.3", + expected: semver.Must(EnsureSemver("12.0.3")), + assertErr: require.NoError, + }, + { + name: "all good with leading v", + statusCode: http.StatusOK, + response: "v12.0.3", + expected: semver.Must(EnsureSemver("12.0.3")), assertErr: require.NoError, }, { name: "non-semver", statusCode: http.StatusOK, response: "hello", - expected: "", + expected: nil, assertErr: func(t2 require.TestingT, err2 error, _ ...interface{}) { require.IsType(t2, &trace.BadParameterError{}, trace.Unwrap(err2)) }, @@ -75,7 +83,7 @@ func Test_basicHTTPVersionClient_Get(t *testing.T) { name: "empty", statusCode: http.StatusOK, response: "", - expected: "", + expected: nil, assertErr: func(t2 require.TestingT, err2 error, _ ...interface{}) { require.IsType(t2, &trace.BadParameterError{}, trace.Unwrap(err2)) }, @@ -84,7 +92,7 @@ func Test_basicHTTPVersionClient_Get(t *testing.T) { name: "non-200 response", statusCode: http.StatusInternalServerError, response: "ERROR - SOMETHING WENT WRONG", - expected: "", + expected: nil, assertErr: require.Error, }, } diff --git a/lib/automaticupgrades/version/errors.go b/lib/automaticupgrades/version/errors.go index 96344c60858de..c8fbb63328c44 100644 --- a/lib/automaticupgrades/version/errors.go +++ b/lib/automaticupgrades/version/errors.go @@ -18,13 +18,17 @@ package version -import "fmt" +import ( + "fmt" + + "github.com/coreos/go-semver/semver" +) // NoNewVersionError indicates that no new version was found and the controller did not reconcile. type NoNewVersionError struct { - Message string `json:"message"` - CurrentVersion string `json:"currentVersion"` - NextVersion string `json:"nextVersion"` + Message string `json:"message"` + CurrentVersion *semver.Version `json:"currentVersion"` + NextVersion *semver.Version `json:"nextVersion"` } // Error returns log friendly description of an error diff --git a/lib/automaticupgrades/version/proxy.go b/lib/automaticupgrades/version/proxy.go index 90ec4859586e2..cb06b8238a713 100644 --- a/lib/automaticupgrades/version/proxy.go +++ b/lib/automaticupgrades/version/proxy.go @@ -21,6 +21,7 @@ package version import ( "context" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/gravitational/teleport/api/client/webclient" @@ -36,14 +37,14 @@ type proxyVersionClient struct { client Finder } -func (b *proxyVersionClient) Get(_ context.Context) (string, error) { +func (b *proxyVersionClient) Get(_ context.Context) (*semver.Version, error) { resp, err := b.client.Find() if err != nil { - return "", trace.Wrap(err) + return nil, trace.Wrap(err) } // We check if a version is advertised to know if the proxy implements RFD-184 or not. if resp.AutoUpdate.AgentVersion == "" { - return "", trace.NotImplemented("proxy does not seem to implement RFD-184") + return nil, trace.NotImplemented("proxy does not seem to implement RFD-184") } return EnsureSemver(resp.AutoUpdate.AgentVersion) } @@ -53,11 +54,11 @@ func (b *proxyVersionClient) Get(_ context.Context) (string, error) { // The Getter returns trace.NotImplementedErr when running against a proxy that does not seem to // expose automatic update instructions over the /find endpoint (proxy too old). type ProxyVersionGetter struct { - cachedGetter func(context.Context) (string, error) + cachedGetter func(context.Context) (*semver.Version, error) } // GetVersion implements Getter -func (g ProxyVersionGetter) GetVersion(ctx context.Context) (string, error) { +func (g ProxyVersionGetter) GetVersion(ctx context.Context) (*semver.Version, error) { return g.cachedGetter(ctx) } @@ -68,5 +69,5 @@ func NewProxyVersionGetter(clt *webclient.ReusableClient) Getter { client: clt, } - return ProxyVersionGetter{cache.NewTimedMemoize[string](versionClient.Get, constants.CacheDuration).Get} + return ProxyVersionGetter{cache.NewTimedMemoize[*semver.Version](versionClient.Get, constants.CacheDuration).Get} } diff --git a/lib/automaticupgrades/version/proxy_test.go b/lib/automaticupgrades/version/proxy_test.go index 2360f271c25a1..e4a4ec8d5160e 100644 --- a/lib/automaticupgrades/version/proxy_test.go +++ b/lib/automaticupgrades/version/proxy_test.go @@ -22,6 +22,7 @@ import ( "context" "testing" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -44,7 +45,7 @@ func TestProxyVersionClient(t *testing.T) { name string pong *webclient.PingResponse pongErr error - expectedVersion string + expectedVersion *semver.Version expectErr require.ErrorAssertionFunc }{ { @@ -54,7 +55,7 @@ func TestProxyVersionClient(t *testing.T) { AgentVersion: "1.2.3", }, }, - expectedVersion: "v1.2.3", + expectedVersion: semver.Must(EnsureSemver("1.2.3")), expectErr: require.NoError, }, { @@ -64,7 +65,7 @@ func TestProxyVersionClient(t *testing.T) { AgentVersion: "v1.2.3", }, }, - expectedVersion: "v1.2.3", + expectedVersion: semver.Must(EnsureSemver("1.2.3")), expectErr: require.NoError, }, { @@ -74,7 +75,7 @@ func TestProxyVersionClient(t *testing.T) { AgentVersion: "1.2.3-dev.bartmoss.1", }, }, - expectedVersion: "v1.2.3-dev.bartmoss.1", + expectedVersion: semver.Must(EnsureSemver("v1.2.3-dev.bartmoss.1")), expectErr: require.NoError, }, { @@ -84,13 +85,13 @@ func TestProxyVersionClient(t *testing.T) { AgentVersion: "v", }, }, - expectedVersion: "", + expectedVersion: nil, expectErr: require.Error, }, { name: "empty response", pong: &webclient.PingResponse{}, - expectedVersion: "", + expectedVersion: nil, expectErr: func(t require.TestingT, err error, i ...interface{}) { require.ErrorIs(t, err, trace.NotImplemented("proxy does not seem to implement RFD-184")) }, diff --git a/lib/automaticupgrades/version/static.go b/lib/automaticupgrades/version/static.go index 1fc4e6798186c..f35252107afaa 100644 --- a/lib/automaticupgrades/version/static.go +++ b/lib/automaticupgrades/version/static.go @@ -21,7 +21,9 @@ package version import ( "context" "fmt" - "strings" + + "github.com/coreos/go-semver/semver" + "github.com/gravitational/trace" "github.com/gravitational/teleport/lib/automaticupgrades/constants" ) @@ -29,30 +31,38 @@ import ( // StaticGetter is a fake version.Getter that return a static answer. This is used // for testing purposes. type StaticGetter struct { - version string + version *semver.Version err error } // GetVersion returns the statically defined version. -func (v StaticGetter) GetVersion(_ context.Context) (string, error) { +func (v StaticGetter) GetVersion(_ context.Context) (*semver.Version, error) { return v.version, v.err } // NewStaticGetter creates a StaticGetter -func NewStaticGetter(version string, err error) Getter { +func NewStaticGetter(version string, err error) (Getter, error) { if version == constants.NoVersion { return StaticGetter{ - version: "", + version: nil, err: &NoNewVersionError{Message: fmt.Sprintf("target version set to '%s'", constants.NoVersion)}, + }, nil + } + + if version == "" { + // If there's no version set but a non-nil error, we are mocking an error and that's OK + if err != nil { + return StaticGetter{nil, err}, nil } + return nil, trace.BadParameter("cannot build a static version getter from an empty version") } - semVersion := version - if semVersion != "" && !strings.HasPrefix(semVersion, "v") { - semVersion = "v" + version + semVersion, err := EnsureSemver(version) + if err != nil { + return nil, trace.Wrap(err) } return StaticGetter{ version: semVersion, err: err, - } + }, nil } diff --git a/lib/automaticupgrades/version/versionget.go b/lib/automaticupgrades/version/versionget.go index e2a1a893e5270..d6aea731a2b7c 100644 --- a/lib/automaticupgrades/version/versionget.go +++ b/lib/automaticupgrades/version/versionget.go @@ -22,9 +22,8 @@ import ( "context" "strings" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" - "golang.org/x/mod/semver" - ctrllog "sigs.k8s.io/controller-runtime/pkg/log" ) // Getter gets the target image version for an external source. It should cache @@ -33,7 +32,7 @@ import ( // If the version source intentionally returns no version, a NoNewVersionError is // returned. type Getter interface { - GetVersion(context.Context) (string, error) + GetVersion(context.Context) (*semver.Version, error) } // FailoverGetter wraps multiple Getters and tries them sequentially. @@ -45,7 +44,7 @@ type FailoverGetter []Getter // GetVersion implements Getter // Getters are evaluated sequentially, the result of the first getter not returning // trace.NotImplementedErr is used. -func (f FailoverGetter) GetVersion(ctx context.Context) (string, error) { +func (f FailoverGetter) GetVersion(ctx context.Context) (*semver.Version, error) { for _, getter := range f { version, err := getter.GetVersion(ctx) switch { @@ -54,27 +53,23 @@ func (f FailoverGetter) GetVersion(ctx context.Context) (string, error) { case trace.IsNotImplemented(err): continue default: - return "", trace.Wrap(err) + return nil, trace.Wrap(err) } } - return "", trace.NotFound("every versionGetter returned NotImplemented") + return nil, trace.NotFound("every versionGetter returned NotImplemented") } // ValidVersionChange receives the current version and the candidate next version // and evaluates if the version transition is valid. -func ValidVersionChange(ctx context.Context, current, next string) bool { - log := ctrllog.FromContext(ctx).V(1) - // Cannot upgrade to a non-valid version - if !semver.IsValid(next) { - log.Error( - trace.BadParameter("next version is not following semver"), - "version change is invalid", - "current_version", current, - "next_version", next, - ) +func ValidVersionChange(ctx context.Context, current, next *semver.Version) bool { + if next == nil { return false } - switch semver.Compare(next, current) { + if current == nil { + return true + } + + switch next.Compare(*current) { // No need to upgrade if version is the same case 0: return false @@ -85,12 +80,12 @@ func ValidVersionChange(ctx context.Context, current, next string) bool { // EnsureSemver adds the 'v' prefix if needed and ensures the provided version // is semver-compliant. -func EnsureSemver(current string) (string, error) { - if !strings.HasPrefix(current, "v") { - current = "v" + current - } - if !semver.IsValid(current) { - return "", trace.BadParameter("tag %s is not following semver", current) +func EnsureSemver(current string) (*semver.Version, error) { + current = strings.TrimPrefix(strings.TrimSpace(current), "v") + + version, err := semver.NewVersion(current) + if err != nil { + return nil, trace.BadParameter("version %s is not following semver", current) } - return current, nil + return version, nil } diff --git a/lib/automaticupgrades/version/versionget_test.go b/lib/automaticupgrades/version/versionget_test.go index 78f4940db229a..375208e827ab4 100644 --- a/lib/automaticupgrades/version/versionget_test.go +++ b/lib/automaticupgrades/version/versionget_test.go @@ -22,23 +22,22 @@ import ( "context" "testing" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/stretchr/testify/require" ) -const ( - semverLow = "v11.3.2" - semverMid = "v11.5.4" - semverHigh = "v12.2.1" - invalidSemverHigh = "12.2.1" +var ( + semverMid = semver.Must(EnsureSemver("v11.5.4")) + semverHigh = semver.Must(EnsureSemver("v12.2.1")) ) func TestValidVersionChange(t *testing.T) { ctx := context.Background() tests := []struct { name string - current string - next string + current *semver.Version + next *semver.Version want bool }{ { @@ -55,14 +54,20 @@ func TestValidVersionChange(t *testing.T) { }, { name: "unknown current version", - current: "", + current: nil, next: semverMid, want: true, }, { name: "non-semver current version", + current: nil, + next: semverHigh, + want: true, + }, + { + name: "non-semver next version", current: semverMid, - next: invalidSemverHigh, + next: nil, want: false, }, } @@ -90,19 +95,19 @@ func TestFailoverGetter_GetVersion(t *testing.T) { tests := []struct { name string getters []Getter - expectResult string + expectResult *semver.Version expectErr require.ErrorAssertionFunc }{ { name: "nil", getters: nil, - expectResult: "", + expectResult: nil, expectErr: checkTraceError(trace.IsNotFound), }, { name: "empty", getters: []Getter{}, - expectResult: "", + expectResult: nil, expectErr: checkTraceError(trace.IsNotFound), }, { @@ -120,7 +125,7 @@ func TestFailoverGetter_GetVersion(t *testing.T) { StaticGetter{err: trace.LimitExceeded("got rate-limited")}, StaticGetter{version: semverHigh}, }, - expectResult: "", + expectResult: nil, expectErr: checkTraceError(trace.IsLimitExceeded), }, { @@ -138,7 +143,7 @@ func TestFailoverGetter_GetVersion(t *testing.T) { StaticGetter{err: trace.NotImplemented("proxy does not seem to implement RFD-184")}, StaticGetter{err: trace.LimitExceeded("got rate-limited")}, }, - expectResult: "", + expectResult: nil, expectErr: checkTraceError(trace.IsLimitExceeded), }, { @@ -147,7 +152,7 @@ func TestFailoverGetter_GetVersion(t *testing.T) { StaticGetter{err: trace.NotImplemented("proxy does not seem to implement RFD-184")}, StaticGetter{err: trace.NotImplemented("proxy does not seem to implement RFD-184")}, }, - expectResult: "", + expectResult: nil, expectErr: checkTraceError(trace.IsNotFound), }, } diff --git a/lib/kube/utils/utils.go b/lib/kube/utils/utils.go index 0806453e356f1..f28f040c6c12f 100644 --- a/lib/kube/utils/utils.go +++ b/lib/kube/utils/utils.go @@ -22,8 +22,8 @@ import ( "context" "encoding/hex" "errors" - "strings" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" @@ -183,21 +183,32 @@ type Pinger interface { // GetKubeAgentVersion returns a version of the Kube agent appropriate for this Teleport cluster. Used for example when deciding version // for enrolling EKS clusters. -func GetKubeAgentVersion(ctx context.Context, pinger Pinger, clusterFeatures proto.Features, versionGetter version.Getter) (string, error) { +func GetKubeAgentVersion(ctx context.Context, pinger Pinger, clusterFeatures proto.Features, versionGetter version.Getter) (*semver.Version, error) { pingResponse, err := pinger.Ping(ctx) if err != nil { - return "", trace.Wrap(err) + return nil, trace.Wrap(err) } - agentVersion := pingResponse.ServerVersion + var agentVersion *semver.Version + + // TODO(hugoShaka) remove the conditional check, we always use the cluster version if clusterFeatures.GetAutomaticUpgrades() && clusterFeatures.GetCloud() { defaultVersion, err := versionGetter.GetVersion(ctx) if err == nil { agentVersion = defaultVersion } else if !errors.Is(err, &version.NoNewVersionError{}) { - return "", trace.Wrap(err) + return nil, trace.Wrap(err) + } + } + + if agentVersion == nil { + clusterVersion, err := version.EnsureSemver(pingResponse.ServerVersion) + if err != nil { + return nil, trace.Wrap(err, "failed to parse cluster version") + } else { + agentVersion = clusterVersion } } - return strings.TrimPrefix(agentVersion, "v"), nil + return agentVersion, nil } diff --git a/lib/kube/utils/utils_test.go b/lib/kube/utils/utils_test.go index b7df332c51c07..145e7946d6bb7 100644 --- a/lib/kube/utils/utils_test.go +++ b/lib/kube/utils/utils_test.go @@ -22,11 +22,13 @@ import ( "context" "testing" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/lib/automaticupgrades" + "github.com/gravitational/teleport/lib/automaticupgrades/version" ) func TestGetAgentVersion(t *testing.T) { @@ -39,7 +41,7 @@ func TestGetAgentVersion(t *testing.T) { ping func(ctx context.Context) (proto.PingResponse, error) clusterFeatures proto.Features channelVersion string - expectedVersion string + expectedVersion *semver.Version errorAssert require.ErrorAssertionFunc }{ { @@ -47,7 +49,7 @@ func TestGetAgentVersion(t *testing.T) { ping: func(ctx context.Context) (proto.PingResponse, error) { return proto.PingResponse{}, trace.BadParameter("ping error") }, - expectedVersion: "", + expectedVersion: nil, errorAssert: require.Error, }, { @@ -55,7 +57,7 @@ func TestGetAgentVersion(t *testing.T) { ping: func(ctx context.Context) (proto.PingResponse, error) { return proto.PingResponse{ServerVersion: "1.2.3"}, nil }, - expectedVersion: "1.2.3", + expectedVersion: semver.Must(version.EnsureSemver("1.2.3")), errorAssert: require.NoError, }, { @@ -65,7 +67,7 @@ func TestGetAgentVersion(t *testing.T) { }, clusterFeatures: proto.Features{AutomaticUpgrades: true, Cloud: true}, channelVersion: "v1.2.3", - expectedVersion: "1.2.3", + expectedVersion: semver.Must(version.EnsureSemver("1.2.3")), errorAssert: require.NoError, }, } @@ -73,7 +75,6 @@ func TestGetAgentVersion(t *testing.T) { for _, tt := range testCases { t.Run(tt.desc, func(t *testing.T) { p := &pinger{pingFn: tt.ping} - var channel *automaticupgrades.Channel if tt.channelVersion != "" { channel = &automaticupgrades.Channel{StaticVersion: tt.channelVersion} diff --git a/lib/service/awsoidc.go b/lib/service/awsoidc.go index 949e59b9144df..2a8020d9a27bd 100644 --- a/lib/service/awsoidc.go +++ b/lib/service/awsoidc.go @@ -25,6 +25,7 @@ import ( "strings" "time" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/jonboulle/clockwork" "golang.org/x/sync/semaphore" @@ -197,12 +198,9 @@ func (updater *AWSOIDCDeployServiceUpdater) updateAWSOIDCDeployServices(ctx cont if err != nil { return trace.Wrap(err) } - // stableVersion has vX.Y.Z format, however the container image tag does not include the `v`. - stableVersion = strings.TrimPrefix(stableVersion, "v") - // minServerVersion specifies the minimum version of the cluster required for // updated AWS OIDC deploy service to remain compatible with the cluster. - minServerVersion, err := utils.MajorSemver(stableVersion) + minServerVersion, err := utils.MajorSemver(stableVersion.String()) if err != nil { return trace.Wrap(err) } @@ -253,11 +251,14 @@ func (updater *AWSOIDCDeployServiceUpdater) updateAWSOIDCDeployServices(ctx cont return trace.Wrap(sem.Acquire(ctx, maxConcurrentUpdates)) } -func (updater *AWSOIDCDeployServiceUpdater) updateAWSOIDCDeployService(ctx context.Context, integration types.Integration, awsRegion, teleportVersion string) error { +func (updater *AWSOIDCDeployServiceUpdater) updateAWSOIDCDeployService(ctx context.Context, integration types.Integration, awsRegion string, teleportVersion *semver.Version) error { // Do not attempt update if integration is not an AWS OIDC integration. if integration.GetAWSOIDCIntegrationSpec() == nil { return nil } + if teleportVersion == nil { + return trace.BadParameter("teleport version is required") + } token, err := updater.AuthClient.GenerateAWSOIDCToken(ctx, integration.GetName()) if err != nil { @@ -313,7 +314,7 @@ func (updater *AWSOIDCDeployServiceUpdater) updateAWSOIDCDeployService(ctx conte ) if err := awsoidc.UpdateDeployService(ctx, awsOIDCDeployServiceClient, updater.Log, awsoidc.UpdateServiceRequest{ TeleportClusterName: updater.TeleportClusterName, - TeleportVersionTag: teleportVersion, + TeleportVersionTag: teleportVersion.String(), OwnershipTags: ownershipTags, }); err != nil { diff --git a/lib/service/service.go b/lib/service/service.go index 06c9aac16faa9..3da7fb1700424 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -47,6 +47,7 @@ import ( "testing" "time" + "github.com/coreos/go-semver/semver" "github.com/google/renameio/v2" "github.com/google/uuid" "github.com/gravitational/trace" @@ -167,7 +168,6 @@ import ( "github.com/gravitational/teleport/lib/utils/cert" "github.com/gravitational/teleport/lib/utils/hostid" logutils "github.com/gravitational/teleport/lib/utils/log" - vc "github.com/gravitational/teleport/lib/versioncontrol" "github.com/gravitational/teleport/lib/versioncontrol/endpoint" uw "github.com/gravitational/teleport/lib/versioncontrol/upgradewindow" "github.com/gravitational/teleport/lib/web" @@ -1240,18 +1240,27 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) { return nil, trace.Wrap(err) } + hello := proto.UpstreamInventoryHello{ + ServerID: cfg.HostUUID, + Version: teleport.Version, + Services: process.getInstanceRoles(), + Hostname: cfg.Hostname, + } + upgraderKind, externalUpgrader, upgraderVersion := process.detectUpgrader() + hello.ExternalUpgrader = externalUpgrader + if upgraderVersion != nil { + // The UpstreamInventoryHello message wants versions with the leading "v". + hello.ExternalUpgraderVersion = "v" + upgraderVersion.String() + } // note: we must create the inventory handle *after* registerExpectedServices because that function determines // the list of services (instance roles) to be included in the heartbeat. - process.inventoryHandle = inventory.NewDownstreamHandle(process.makeInventoryControlStreamWhenReady, proto.UpstreamInventoryHello{ - ServerID: cfg.HostUUID, - Version: teleport.Version, - Services: process.getInstanceRoles(), - Hostname: cfg.Hostname, - ExternalUpgrader: externalUpgrader, - ExternalUpgraderVersion: vc.Normalize(upgraderVersion), - }, inventory.WithDownstreamClock(process.Clock)) + process.inventoryHandle = inventory.NewDownstreamHandle( + process.makeInventoryControlStreamWhenReady, + hello, + inventory.WithDownstreamClock(process.Clock), + ) process.inventoryHandle.RegisterPingHandler(func(sender inventory.DownstreamSender, ping proto.DownstreamInventoryPing) { systemClock := process.Clock.Now().UTC() @@ -1545,11 +1554,11 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) { // Note that kind and externalName are usually the same. // However, some unregistered upgraders like the AWS ODIC upgrader are not valid kinds. // For these upgraders, kind is empty and externalName is set to a non-kind value. -func (process *TeleportProcess) detectUpgrader() (kind, externalName, version string) { +func (process *TeleportProcess) detectUpgrader() (kind, externalName string, version *semver.Version) { // Check if the deprecated teleport-upgrader script is being used. kind = os.Getenv(automaticupgrades.EnvUpgrader) version = automaticupgrades.GetUpgraderVersion(process.GracefulExitContext()) - if version == "" { + if version == nil { kind = "" } @@ -1561,7 +1570,7 @@ func (process *TeleportProcess) detectUpgrader() (kind, externalName, version st // If this is a teleport-update managed installation, the version // managed by the timer will always match the installed version of teleport. kind = types.UpgraderKindTeleportUpdate - version = "v" + teleport.Version + version = teleport.SemVersion } // Instances deployed using the AWS OIDC integration are automatically updated diff --git a/lib/srv/discovery/kube_integration_watcher.go b/lib/srv/discovery/kube_integration_watcher.go index 0d5fe0ba88198..d5e371cbbb7d1 100644 --- a/lib/srv/discovery/kube_integration_watcher.go +++ b/lib/srv/discovery/kube_integration_watcher.go @@ -28,6 +28,7 @@ import ( "sync" "time" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "google.golang.org/protobuf/types/known/timestamppb" @@ -72,7 +73,10 @@ func (s *Server) startKubeIntegrationWatchers() error { s.Log.WarnContext(s.ctx, "Failed to determine proxy public address, agents will install our own Teleport version instead of the one advertised by the proxy.", "version", teleport.Version) - versionGetter = version.NewStaticGetter(teleport.Version, nil) + versionGetter, err = version.NewStaticGetter(teleport.Version, nil) + if err != nil { + return trace.BadParameter("Cannot parse Teleport's self version %q, this is a bug", teleport.Version) + } } else { versionGetter, err = versionGetterForProxy(s.ctx, proxyPublicAddr) if err != nil { @@ -80,7 +84,10 @@ func (s *Server) startKubeIntegrationWatchers() error { "Failed to build a version client, falling back to Discovery service Teleport version.", "error", err, "version", teleport.Version) - versionGetter = version.NewStaticGetter(teleport.Version, nil) + versionGetter, err = version.NewStaticGetter(teleport.Version, nil) + if err != nil { + return trace.BadParameter("Cannot parse Teleport's self version %q, this is a bug", teleport.Version) + } } } @@ -229,7 +236,7 @@ func (s *Server) kubernetesIntegrationWatcherIterationStarted() { s.awsEKSTasks.reset() } -func (s *Server) enrollEKSClusters(region, integration, discoveryConfigName string, clusters []types.DiscoveredEKSCluster, agentVersion string, mu *sync.Mutex, enrollingClusters map[string]bool) { +func (s *Server) enrollEKSClusters(region, integration, discoveryConfigName string, clusters []types.DiscoveredEKSCluster, agentVersion *semver.Version, mu *sync.Mutex, enrollingClusters map[string]bool) { mu.Lock() for _, c := range clusters { if _, ok := enrollingClusters[c.GetAWSConfig().Name]; !ok { @@ -272,7 +279,7 @@ func (s *Server) enrollEKSClusters(region, integration, discoveryConfigName stri Region: region, EksClusterNames: clusterNames, EnableAppDiscovery: kubeAppDiscovery, - AgentVersion: agentVersion, + AgentVersion: agentVersion.String(), }) if err != nil { s.awsEKSResourcesStatus.incrementFailed(awsResourceGroup{ @@ -323,7 +330,7 @@ func (s *Server) enrollEKSClusters(region, integration, discoveryConfigName stri } } -func (s *Server) getKubeAgentVersion(versionGetter version.Getter) (string, error) { +func (s *Server) getKubeAgentVersion(versionGetter version.Getter) (*semver.Version, error) { return kubeutils.GetKubeAgentVersion(s.ctx, s.AccessPoint, s.ClusterFeatures(), versionGetter) } diff --git a/lib/srv/server/installer/autodiscover.go b/lib/srv/server/installer/autodiscover.go index c1ae6a4a1db5a..930b096f8360f 100644 --- a/lib/srv/server/installer/autodiscover.go +++ b/lib/srv/server/installer/autodiscover.go @@ -462,6 +462,7 @@ func (ani *AutoDiscoverNodeInstaller) fetchTargetVersion(ctx context.Context) st return api.Version } + // TODO(hugoShaka): convert this to a proxy version getter targetVersion, err := version.NewBasicHTTPVersionGetter(upgradeURL).GetVersion(ctx) if err != nil { ani.Logger.WarnContext(ctx, "Failed to query target version, using api version", @@ -473,7 +474,7 @@ func (ani *AutoDiscoverNodeInstaller) fetchTargetVersion(ctx context.Context) st "channel_url", ani.autoUpgradesChannelURL, "version", targetVersion) - return strings.TrimSpace(strings.TrimPrefix(targetVersion, "v")) + return strings.TrimSpace(strings.TrimPrefix(targetVersion.String(), "v")) } func fetchNodeAutoDiscoverLabels(ctx context.Context, imdsClient imds.Client) (map[string]string, error) { diff --git a/lib/utils/teleportassets/teleportassets.go b/lib/utils/teleportassets/teleportassets.go index 302977a8bd9ef..995f035676293 100644 --- a/lib/utils/teleportassets/teleportassets.go +++ b/lib/utils/teleportassets/teleportassets.go @@ -60,6 +60,11 @@ func CDNBaseURLForVersion(artifactVersion *semver.Version) string { } func cdnBaseURLForVersion(artifactVersion, teleportVersion *semver.Version) string { + // Something is not right a version is nil, we default to the standard CDN. + if artifactVersion == nil || teleportVersion == nil { + return TeleportReleaseCDN + } + if teleportVersion.PreRelease != "" && artifactVersion.PreRelease != "" { return teleportPreReleaseCDN } diff --git a/lib/versioncontrol/github/github.go b/lib/versioncontrol/github/github.go index 0d767c3d012e4..f3c2f6ea83c24 100644 --- a/lib/versioncontrol/github/github.go +++ b/lib/versioncontrol/github/github.go @@ -28,7 +28,7 @@ import ( "strings" "github.com/gravitational/trace" - "golang.org/x/mod/semver" + "golang.org/x/mod/semver" //nolint:depguard // Usage precedes the x/mod/semver rule. "github.com/gravitational/teleport" logutils "github.com/gravitational/teleport/lib/utils/log" diff --git a/lib/versioncontrol/target.go b/lib/versioncontrol/target.go index f2f41136ba163..6f034fb4f9eb3 100644 --- a/lib/versioncontrol/target.go +++ b/lib/versioncontrol/target.go @@ -24,7 +24,7 @@ import ( "strconv" "strings" - "golang.org/x/mod/semver" + "golang.org/x/mod/semver" //nolint:depguard // Usage precedes the x/mod/semver rule. ) // NOTE: the contents of this file might be moving to the 'api' package in the future. diff --git a/lib/versioncontrol/versioncontrol.go b/lib/versioncontrol/versioncontrol.go index 6ec5966999ed2..dece538816527 100644 --- a/lib/versioncontrol/versioncontrol.go +++ b/lib/versioncontrol/versioncontrol.go @@ -21,7 +21,7 @@ package versioncontrol import ( "fmt" - "golang.org/x/mod/semver" + "golang.org/x/mod/semver" //nolint:depguard // Usage precedes the x/mod/semver rule. ) // Normalize attaches the expected `v` prefix to a version string if the supplied diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 4e6632e294f4e..84935a452afc1 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -54,7 +54,6 @@ import ( oteltrace "go.opentelemetry.io/otel/trace" tracepb "go.opentelemetry.io/proto/otlp/trace/v1" "golang.org/x/crypto/ssh" - "golang.org/x/mod/semver" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/types/known/timestamppb" @@ -2228,8 +2227,14 @@ func (h *Handler) installer(w http.ResponseWriter, r *http.Request, p httprouter if err != nil { return nil, trace.Wrap(err) } - // semver parsing requires a 'v' at the beginning of the version string. - version := semver.Major("v" + ping.ServerVersion) + + const group, agentUUD = "", "" + targetVersion, err := h.autoUpdateAgentVersion(r.Context(), group, agentUUD) + if err != nil { + h.logger.WarnContext(r.Context(), "Error retrieving the target version", "error", err) + targetVersion = teleport.SemVersion + } + instTmpl, err := template.New("").Parse(installer.GetScript()) if err != nil { return nil, trace.Wrap(err) @@ -2245,7 +2250,7 @@ func (h *Handler) installer(w http.ResponseWriter, r *http.Request, p httprouter } // By default, it uses the stable/v channel. - repoChannel := fmt.Sprintf("stable/%s", version) + repoChannel := fmt.Sprintf("stable/v%d", targetVersion.Major) // If the updater must be installed, then change the repo to stable/cloud // It must also install the version specified in @@ -2258,7 +2263,7 @@ func (h *Handler) installer(w http.ResponseWriter, r *http.Request, p httprouter tmpl := installers.Template{ PublicProxyAddr: h.PublicProxyAddr(), - MajorVersion: shsprintf.EscapeDefaultContext(version), + MajorVersion: shsprintf.EscapeDefaultContext(fmt.Sprintf("v%d", targetVersion.Major)), TeleportPackage: teleportPackage, RepoChannel: shsprintf.EscapeDefaultContext(repoChannel), AutomaticUpgrades: strconv.FormatBool(installUpdater), diff --git a/lib/web/autoupdate_common.go b/lib/web/autoupdate_common.go index 0daaadaec02ce..b65cd7099fb8f 100644 --- a/lib/web/autoupdate_common.go +++ b/lib/web/autoupdate_common.go @@ -20,15 +20,15 @@ package web import ( "context" - "fmt" - "strings" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" autoupdatepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/autoupdate" "github.com/gravitational/teleport/lib/automaticupgrades" + "github.com/gravitational/teleport/lib/automaticupgrades/version" "github.com/gravitational/teleport/lib/utils" ) @@ -37,8 +37,7 @@ import ( // If the cluster contains an autoupdate_agent_rollout resource from RFD184 it should take precedence. // If the resource is not there, we fall back to RFD109-style updates with channels // and maintenance window derived from the cluster_maintenance_config resource. -// Version returned follows semver without the leading "v". -func (h *Handler) autoUpdateAgentVersion(ctx context.Context, group, updaterUUID string) (string, error) { +func (h *Handler) autoUpdateAgentVersion(ctx context.Context, group, updaterUUID string) (*semver.Version, error) { rollout, err := h.cfg.AccessPoint.GetAutoUpdateAgentRollout(ctx) if err != nil { // Fallback to channels if there is no autoupdate_agent_rollout. @@ -46,7 +45,7 @@ func (h *Handler) autoUpdateAgentVersion(ctx context.Context, group, updaterUUID return getVersionFromChannel(ctx, h.cfg.AutomaticUpgradesChannels, group) } // Something is broken, we don't want to fallback to channels, this would be harmful. - return "", trace.Wrap(err, "getting autoupdate_agent_rollout") + return nil, trace.Wrap(err, "getting autoupdate_agent_rollout") } return getVersionFromRollout(rollout, group, updaterUUID) @@ -58,14 +57,10 @@ type handlerVersionGetter struct { } // GetVersion implements version.Getter. -func (h *handlerVersionGetter) GetVersion(ctx context.Context) (string, error) { +func (h *handlerVersionGetter) GetVersion(ctx context.Context) (*semver.Version, error) { const group, updaterUUID = "", "" agentVersion, err := h.autoUpdateAgentVersion(ctx, group, updaterUUID) - if err != nil { - return "", trace.Wrap(err) - } - // We add the leading v required by the version.Getter interface. - return fmt.Sprintf("v%s", agentVersion), nil + return agentVersion, trace.Wrap(err) } // autoUpdateAgentShouldUpdate returns if the agent should update now to based on its group @@ -98,41 +93,40 @@ func (h *Handler) autoUpdateAgentShouldUpdate(ctx context.Context, group, update // This logic is pretty complex and described in RFD 184. // The spec is summed up in the following table: // https://github.com/gravitational/teleport/blob/master/rfd/0184-agent-auto-updates.md#rollout-status-disabled -// Version returned follows semver without the leading "v". func getVersionFromRollout( rollout *autoupdatepb.AutoUpdateAgentRollout, groupName, updaterUUID string, -) (string, error) { +) (*semver.Version, error) { switch rollout.GetSpec().GetAutoupdateMode() { case autoupdate.AgentsUpdateModeDisabled: // If AUs are disabled, we always answer the target version - return rollout.GetSpec().GetTargetVersion(), nil + return version.EnsureSemver(rollout.GetSpec().GetTargetVersion()) case autoupdate.AgentsUpdateModeSuspended, autoupdate.AgentsUpdateModeEnabled: // If AUs are enabled or suspended, we modulate the response based on the schedule and agent group state default: - return "", trace.BadParameter("unsupported agent update mode %q", rollout.GetSpec().GetAutoupdateMode()) + return nil, trace.BadParameter("unsupported agent update mode %q", rollout.GetSpec().GetAutoupdateMode()) } // If the schedule is immediate, agents always update to the latest version if rollout.GetSpec().GetSchedule() == autoupdate.AgentsScheduleImmediate { - return rollout.GetSpec().GetTargetVersion(), nil + return version.EnsureSemver(rollout.GetSpec().GetTargetVersion()) } // Else we follow the regular schedule and answer based on the agent group state group, err := getGroup(rollout, groupName) if err != nil { - return "", trace.Wrap(err, "getting group %q", groupName) + return nil, trace.Wrap(err, "getting group %q", groupName) } switch group.GetState() { case autoupdatepb.AutoUpdateAgentGroupState_AUTO_UPDATE_AGENT_GROUP_STATE_UNSTARTED, autoupdatepb.AutoUpdateAgentGroupState_AUTO_UPDATE_AGENT_GROUP_STATE_ROLLEDBACK: - return rollout.GetSpec().GetStartVersion(), nil + return version.EnsureSemver(rollout.GetSpec().GetStartVersion()) case autoupdatepb.AutoUpdateAgentGroupState_AUTO_UPDATE_AGENT_GROUP_STATE_ACTIVE, autoupdatepb.AutoUpdateAgentGroupState_AUTO_UPDATE_AGENT_GROUP_STATE_DONE: - return rollout.GetSpec().GetTargetVersion(), nil + return version.EnsureSemver(rollout.GetSpec().GetTargetVersion()) default: - return "", trace.NotImplemented("unsupported group state %q", group.GetState()) + return nil, trace.NotImplemented("unsupported group state %q", group.GetState()) } } @@ -202,14 +196,7 @@ func getGroup( } // getVersionFromChannel gets the target version from the RFD109 channels. -// Version returned follows semver without the leading "v". -func getVersionFromChannel(ctx context.Context, channels automaticupgrades.Channels, groupName string) (version string, err error) { - // RFD109 channels return the version with the 'v' prefix. - // We can't change the internals for backward compatibility, so we must trim the prefix if it's here. - defer func() { - version = strings.TrimPrefix(version, "v") - }() - +func getVersionFromChannel(ctx context.Context, channels automaticupgrades.Channels, groupName string) (version *semver.Version, err error) { if channel, ok := channels[groupName]; ok { return channel.GetVersion(ctx) } diff --git a/lib/web/autoupdate_common_test.go b/lib/web/autoupdate_common_test.go index e0a1a31719586..b84070d115f1d 100644 --- a/lib/web/autoupdate_common_test.go +++ b/lib/web/autoupdate_common_test.go @@ -27,6 +27,7 @@ import ( "testing" "time" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/jonboulle/clockwork" "github.com/stretchr/testify/assert" @@ -38,6 +39,7 @@ import ( "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/automaticupgrades" "github.com/gravitational/teleport/lib/automaticupgrades/constants" + "github.com/gravitational/teleport/lib/automaticupgrades/version" "github.com/gravitational/teleport/lib/utils" ) @@ -90,7 +92,7 @@ func TestAutoUpdateAgentVersion(t *testing.T) { rollout *autoupdatepb.AutoUpdateAgentRollout rolloutErr error channel *automaticupgrades.Channel - expectedVersion string + expectedVersion *semver.Version expectError require.ErrorAssertionFunc }{ { @@ -104,14 +106,14 @@ func TestAutoUpdateAgentVersion(t *testing.T) { }, channel: &automaticupgrades.Channel{StaticVersion: testVersionLow}, expectError: require.NoError, - expectedVersion: testVersionHigh, + expectedVersion: semver.Must(version.EnsureSemver(testVersionHigh)), }, { name: "version is looked up from channel if rollout is not here", rolloutErr: trace.NotFound("rollout is not here"), channel: &automaticupgrades.Channel{StaticVersion: testVersionLow}, expectError: require.NoError, - expectedVersion: testVersionLow, + expectedVersion: semver.Must(version.EnsureSemver(testVersionLow)), }, { name: "hard error getting rollout should not fallback to version channels", @@ -390,6 +392,9 @@ func TestGetVersionFromRollout(t *testing.T) { for schedule, stateCases := range scheduleCases { for state, expectedVersion := range stateCases { t.Run(fmt.Sprintf("%s/%s/%s", mode, schedule, state), func(t *testing.T) { + expectedSemVersion, err := version.EnsureSemver(expectedVersion) + require.NoError(t, err) + rollout := &autoupdatepb.AutoUpdateAgentRollout{ Spec: &autoupdatepb.AutoUpdateAgentRolloutSpec{ StartVersion: testVersionLow, @@ -410,7 +415,7 @@ func TestGetVersionFromRollout(t *testing.T) { } version, err := getVersionFromRollout(rollout, groupName, "") require.NoError(t, err) - require.Equal(t, expectedVersion, version) + require.Equal(t, expectedSemVersion, version) }) } } @@ -657,10 +662,13 @@ func TestGetVersionFromChannel(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(mock.ServeHTTP)) t.Cleanup(srv.Close) + testVersion, err := version.EnsureSemver(testVersionHigh) + require.NoError(t, err) + tests := []struct { name string channels automaticupgrades.Channels - expectedResult string + expectedResult *semver.Version expectError require.ErrorAssertionFunc }{ { @@ -669,7 +677,7 @@ func TestGetVersionFromChannel(t *testing.T) { channelName: {ForwardURL: srv.URL + "/with-leading-v"}, "default": {ForwardURL: srv.URL + "/low"}, }, - expectedResult: testVersionHigh, + expectedResult: testVersion, expectError: require.NoError, }, { @@ -678,7 +686,7 @@ func TestGetVersionFromChannel(t *testing.T) { channelName: {ForwardURL: srv.URL + "/without-leading-v"}, "default": {ForwardURL: srv.URL + "/low"}, }, - expectedResult: testVersionHigh, + expectedResult: testVersion, expectError: require.NoError, }, { @@ -686,7 +694,7 @@ func TestGetVersionFromChannel(t *testing.T) { channels: automaticupgrades.Channels{ "default": {ForwardURL: srv.URL + "/with-leading-v"}, }, - expectedResult: testVersionHigh, + expectedResult: testVersion, expectError: require.NoError, }, { @@ -694,7 +702,7 @@ func TestGetVersionFromChannel(t *testing.T) { channels: automaticupgrades.Channels{ "default": {ForwardURL: srv.URL + "/without-leading-v"}, }, - expectedResult: testVersionHigh, + expectedResult: testVersion, expectError: require.NoError, }, { diff --git a/lib/web/autoupdate_rfd109.go b/lib/web/autoupdate_rfd109.go index d38723a029d95..4817493e995ec 100644 --- a/lib/web/autoupdate_rfd109.go +++ b/lib/web/autoupdate_rfd109.go @@ -92,7 +92,7 @@ func (h *Handler) automaticUpgradesVersion109(w http.ResponseWriter, r *http.Req // RFD 109 specifies that version from channels must have the leading "v". // As h.autoUpdateAgentVersion doesn't, we must add it. - _, err = fmt.Fprintf(w, "v%s", targetVersion) + _, err = fmt.Fprintf(w, "v%s", targetVersion.String()) return nil, trace.Wrap(err) } diff --git a/lib/web/autoupdate_rfd184.go b/lib/web/autoupdate_rfd184.go index 6ac532650cb64..07a65596646c3 100644 --- a/lib/web/autoupdate_rfd184.go +++ b/lib/web/autoupdate_rfd184.go @@ -21,13 +21,14 @@ package web import ( "context" + "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" "github.com/gravitational/teleport" - "github.com/gravitational/teleport/api" "github.com/gravitational/teleport/api/client/webclient" autoupdatepb "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" "github.com/gravitational/teleport/api/types/autoupdate" + "github.com/gravitational/teleport/lib/automaticupgrades/version" ) // automaticUpdateSettings184 crafts the automatic updates part of the ping/find response @@ -51,7 +52,7 @@ func (h *Handler) automaticUpdateSettings184(ctx context.Context, group, updater if err != nil { h.logger.ErrorContext(ctx, "failed to resolve AgentVersion", "error", err) // Defaulting to current version - agentVersion = teleport.Version + agentVersion = teleport.SemVersion } // If the source of truth is RFD 109 configuration (channels + CMC) we must emulate the // RFD109 agent maintenance window behavior by looking up the CMC and checking if @@ -63,11 +64,17 @@ func (h *Handler) automaticUpdateSettings184(ctx context.Context, group, updater shouldUpdate = false } + toolsVersion, err := getToolsVersion(autoUpdateVersion) + if err != nil { + h.logger.ErrorContext(ctx, "failed to get rools version", "error", err) + toolsVersion = teleport.SemVersion + } + return webclient.AutoUpdateSettings{ ToolsAutoUpdate: getToolsAutoUpdate(autoUpdateConfig), - ToolsVersion: getToolsVersion(autoUpdateVersion), + ToolsVersion: toolsVersion.String(), AgentUpdateJitterSeconds: DefaultAgentUpdateJitterSeconds, - AgentVersion: agentVersion, + AgentVersion: agentVersion.String(), AgentAutoUpdate: shouldUpdate, } } @@ -83,11 +90,11 @@ func getToolsAutoUpdate(config *autoupdatepb.AutoUpdateConfig) bool { return false } -func getToolsVersion(version *autoupdatepb.AutoUpdateVersion) string { +func getToolsVersion(v *autoupdatepb.AutoUpdateVersion) (*semver.Version, error) { // If we can't get the AU version or tools AU version is not specified, we default to the current proxy version. // This ensures we always advertise a version compatible with the cluster. - if version.GetSpec().GetTools() == nil { - return api.Version + if v.GetSpec().GetTools() == nil { + return teleport.SemVersion, nil } - return version.GetSpec().GetTools().GetTargetVersion() + return version.EnsureSemver(v.GetSpec().GetTools().GetTargetVersion()) } diff --git a/lib/web/integrations_awsoidc.go b/lib/web/integrations_awsoidc.go index ff98c9ed8f1dd..eb36e3700eb9e 100644 --- a/lib/web/integrations_awsoidc.go +++ b/lib/web/integrations_awsoidc.go @@ -166,7 +166,7 @@ func (h *Handler) awsOIDCDeployService(w http.ResponseWriter, r *http.Request, p "error", err, "version", teleport.Version) } else { - teleportVersionTag = autoUpdateVersion + teleportVersionTag = autoUpdateVersion.String() } } @@ -222,7 +222,7 @@ func (h *Handler) awsOIDCDeployDatabaseServices(w http.ResponseWriter, r *http.R "error", err, "version", teleport.Version) } else { - teleportVersionTag = autoUpdateVersion + teleportVersionTag = autoUpdateVersion.String() } } @@ -794,7 +794,7 @@ func (h *Handler) awsOIDCEnrollEKSClusters(w http.ResponseWriter, r *http.Reques Region: req.Region, EksClusterNames: req.ClusterNames, EnableAppDiscovery: req.EnableAppDiscovery, - AgentVersion: agentVersion, + AgentVersion: agentVersion.String(), ExtraLabels: extraLabels, }) if err != nil { diff --git a/lib/web/scripts.go b/lib/web/scripts.go index 54d520084a9f3..79f809185b06e 100644 --- a/lib/web/scripts.go +++ b/lib/web/scripts.go @@ -89,7 +89,7 @@ func (h *Handler) installScriptOptions(ctx context.Context) (scripts.InstallScri version, err := h.autoUpdateAgentVersion(ctx, defaultGroup, defaultUpdater) if err != nil { h.logger.WarnContext(ctx, "Failed to get intended agent version", "error", err) - version = teleport.Version + version = teleport.SemVersion } // if there's a rollout, we do new autoupdates @@ -145,17 +145,12 @@ func (h *Handler) installScriptOptions(ctx context.Context) (scripts.InstallScri // - "https://cdn.cloud.gravitational.io" (dev builds/staging) const EnvVarCDNBaseURL = "TELEPORT_CDN_BASE_URL" -func getCDNBaseURL(version string) (string, error) { +func getCDNBaseURL(version *semver.Version) (string, error) { // If the user explicitly overrides the CDN base URL, we use it. if override := os.Getenv(EnvVarCDNBaseURL); override != "" { return override, nil } - v, err := semver.NewVersion(version) - if err != nil { - return "", trace.Wrap(err) - } - // If this is an AGPL build, we don't want to automatically install binaries distributed under a more restrictive // license so we error and ask the user set the CDN URL, either to: // - the official Teleport CDN if they agree with the community license and meet its requirements @@ -167,5 +162,5 @@ func getCDNBaseURL(version string) (string, error) { teleportassets.CDNBaseURL()) } - return teleportassets.CDNBaseURLForVersion(v), nil + return teleportassets.CDNBaseURLForVersion(version), nil } diff --git a/lib/web/scripts/install.go b/lib/web/scripts/install.go index 0e6d956be514a..394fb5bc3d035 100644 --- a/lib/web/scripts/install.go +++ b/lib/web/scripts/install.go @@ -26,6 +26,7 @@ import ( "regexp" "strings" + "github.com/coreos/go-semver/semver" "github.com/google/safetext/shsprintf" "github.com/gravitational/trace" @@ -70,7 +71,7 @@ func (style AutoupdateStyle) String() string { type InstallScriptOptions struct { AutoupdateStyle AutoupdateStyle // TeleportVersion that should be installed. Without the leading "v". - TeleportVersion string + TeleportVersion *semver.Version // CDNBaseURL is the URL of the CDN hosting teleport tarballs. // If left empty, the 'teleport-update' installer will pick the one to use. // For example: "https://cdn.example.com" @@ -107,7 +108,7 @@ func (o *InstallScriptOptions) Check() error { return trace.BadParameter("Proxy address is required") } - if o.TeleportVersion == "" { + if o.TeleportVersion == nil { return trace.BadParameter("Teleport version is required") } @@ -130,12 +131,6 @@ func (o *InstallScriptOptions) Check() error { // oneOffParams returns the oneoff.OneOffScriptParams that will install Teleport // using the oneoff.sh script to download and execute 'teleport-update'. func (o *InstallScriptOptions) oneOffParams() (params oneoff.OneOffScriptParams) { - // We add the leading v if it's not here - version := o.TeleportVersion - if o.TeleportVersion[0] != 'v' { - version = "v" + o.TeleportVersion - } - args := []string{"enable", "--proxy", shsprintf.EscapeDefaultContext(o.ProxyAddr)} // Pass the base-url override if the base url is set and is not the default one. if o.CDNBaseURL != "" && o.CDNBaseURL != teleportUpdateDefaultCDN { @@ -152,7 +147,7 @@ func (o *InstallScriptOptions) oneOffParams() (params oneoff.OneOffScriptParams) Entrypoint: "teleport-update", EntrypointArgs: strings.Join(args, " "), CDNBaseURL: o.CDNBaseURL, - TeleportVersion: version, + TeleportVersion: "v" + o.TeleportVersion.String(), TeleportFlavor: o.TeleportFlavor, SuccessMessage: successMessage, TeleportFIPS: o.FIPS, diff --git a/lib/web/scripts/install_node.go b/lib/web/scripts/install_node.go index e99aa5701a2ac..c38cd5f822667 100644 --- a/lib/web/scripts/install_node.go +++ b/lib/web/scripts/install_node.go @@ -172,7 +172,7 @@ func GetNodeInstallScript(ctx context.Context, opts InstallNodeScriptOptions) (s "packageName": opts.InstallOptions.TeleportFlavor, "repoChannel": repoChannel, "installUpdater": opts.InstallOptions.AutoupdateStyle.String(), - "version": shsprintf.EscapeDefaultContext(opts.InstallOptions.TeleportVersion), + "version": shsprintf.EscapeDefaultContext(opts.InstallOptions.TeleportVersion.String()), "appInstallMode": strconv.FormatBool(opts.AppServiceEnabled), "appServerResourceLabels": appServerResourceLabels, "appName": shsprintf.EscapeDefaultContext(opts.AppName), diff --git a/lib/web/scripts/install_test.go b/lib/web/scripts/install_test.go index 5061f5d02bc37..82074daffa5e6 100644 --- a/lib/web/scripts/install_test.go +++ b/lib/web/scripts/install_test.go @@ -26,12 +26,14 @@ import ( "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/automaticupgrades/version" "github.com/gravitational/teleport/lib/utils/teleportassets" ) func TestGetInstallScript(t *testing.T) { ctx := context.Background() - testVersion := "1.2.3" + testVersion, err := version.EnsureSemver("1.2.3") + require.NoError(t, err) testProxyAddr := "proxy.example.com:443" tests := []struct {