fix(appliance): source versions from release registry (#63387)

Craig Furman · web-flow · commit b47c376cbef0 · 2024-06-24T09:48:50.000Z
Rather than hardcoding a few. Present the user with versions up to 2 minor revisions back from the version of the appliance itself, which should be in lock-step with the rest of the monorepo. Closes https://linear.app/sourcegraph/issue/REL-199/populate-accurate-list-of-versions-to-install
diff --git a/cmd/appliance/README.md b/cmd/appliance/README.md
@@ -11,3 +11,34 @@ Appliance runs as a standard Kubernetes Deployment and utilizes Kubernetes [cont
 ## Own
 
 For more information or for help, see the [Release Team](https://handbook.sourcegraph.com/departments/engineering/teams/release/).
+
+## Development
+
+You can kick the tires on the appliance version by running:
+
+```
+go run ./cmd/appliance
+```
+
+[`config.go`](./shared/config.go) is the source of truth on appliance
+configuration. Most of the variables there are optional, except for:
+
+- `APPLIANCE_VERSION`: while this does have a default that does not need to be
+  overridden in production, development builds that lack the link-time injected
+  version information will need to set this. Set it to the latest version of
+  Sourcegraph that you want to be offered.
+
+You might want to override the listen addresses to localhost-only, in order to
+avoid macos firewall popups.
+
+The appliance doesn't care if it's running inside or out of the k8s cluster it's
+provisioning resources into. It does a well-known k8s config dance to try to
+load in-cluster config (from a k8s ServiceAccount token), falling back on
+looking for a kubeconfig on the host
+
+If you have some kubernetes running (e.g. minikube, docker desktop), and your
+default context is set in ~/.kube/config, the appliance will build a k8s client
+using that kubeconfig, and everything should "just work".
+
+See [`development.md`](../..internal/appliance/development.md) for more
+information, including about automated testing.
diff --git a/cmd/appliance/shared/BUILD.bazel b/cmd/appliance/shared/BUILD.bazel
@@ -17,7 +17,9 @@ go_library(
         "//internal/env",
         "//internal/grpc/defaults",
         "//internal/observation",
+        "//internal/releaseregistry",
         "//internal/service",
+        "//internal/version",
         "//lib/errors",
         "@com_github_sourcegraph_log//:log",
         "@com_github_sourcegraph_log_logr//:logr",
diff --git a/cmd/appliance/shared/config.go b/cmd/appliance/shared/config.go
@@ -8,17 +8,21 @@ import (
 	"k8s.io/client-go/util/homedir"
 
 	"github.com/sourcegraph/sourcegraph/internal/env"
+	"github.com/sourcegraph/sourcegraph/internal/releaseregistry"
+	"github.com/sourcegraph/sourcegraph/internal/version"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
 
 type Config struct {
 	env.BaseConfig
 
-	k8sConfig *rest.Config
-	metrics   metricsConfig
-	grpc      grpcConfig
-	http      httpConfig
-	namespace string
+	k8sConfig        *rest.Config
+	metrics          metricsConfig
+	grpc             grpcConfig
+	http             httpConfig
+	namespace        string
+	relregEndpoint   string
+	applianceVersion string
 }
 
 func (c *Config) Load() {
@@ -41,6 +45,8 @@ func (c *Config) Load() {
 	c.grpc.addr = c.Get("APPLIANCE_GRPC_ADDR", ":9000", "Appliance gRPC address.")
 	c.http.addr = c.Get("APPLIANCE_HTTP_ADDR", ":8080", "Appliance http address.")
 	c.namespace = c.Get("APPLIANCE_NAMESPACE", "default", "Namespace to monitor.")
+	c.applianceVersion = c.Get("APPLIANCE_VERSION", version.Version(), "Version tag for the running appliance.")
+	c.relregEndpoint = c.Get("RELEASE_REGISTRY_ENDPOINT", releaseregistry.Endpoint, "Release registry endpoint.")
 }
 
 func (c *Config) Validate() error {
diff --git a/cmd/appliance/shared/shared.go b/cmd/appliance/shared/shared.go
@@ -24,6 +24,7 @@ import (
 	pb "github.com/sourcegraph/sourcegraph/internal/appliance/v1"
 	"github.com/sourcegraph/sourcegraph/internal/grpc/defaults"
 	"github.com/sourcegraph/sourcegraph/internal/observation"
+	"github.com/sourcegraph/sourcegraph/internal/releaseregistry"
 	"github.com/sourcegraph/sourcegraph/internal/service"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
 )
@@ -42,7 +43,9 @@ func Start(ctx context.Context, observationCtx *observation.Context, ready servi
 		return err
 	}
 
-	app := appliance.NewAppliance(k8sClient, config.namespace, logger)
+	relregClient := releaseregistry.NewClient(config.relregEndpoint)
+
+	app := appliance.NewAppliance(k8sClient, relregClient, config.applianceVersion, config.namespace, logger)
 
 	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{
 		Logger: logr,
diff --git a/dev/sg/internal/release/list_release.go b/dev/sg/internal/release/list_release.go
@@ -33,7 +33,7 @@ func listRegistryVersions(cmd *cli.Context) error {
 
 	out := std.NewOutput(os.Stderr, false)
 	pending := out.Pending(output.Line("", output.StylePending, "Fetching versions from the release registry..."))
-	versions, err := client.ListVersions(cmd.Context)
+	versions, err := client.ListVersions(cmd.Context, "")
 	if err != nil {
 		pending.Complete(output.Linef(output.EmojiFailure, output.StyleFailure, "Failed to fetch versions from release registry"))
 		return err
diff --git a/go.mod b/go.mod
@@ -257,6 +257,7 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/ai/azopenai v0.5.0
 	github.com/Azure/azure-sdk-for-go/sdk/azcore v1.9.2
 	github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.4.0
+	github.com/Masterminds/semver/v3 v3.2.1
 	github.com/aws/constructs-go/constructs/v10 v10.3.0
 	github.com/aws/jsii-runtime-go v1.98.0
 	github.com/bazelbuild/bazel-gazelle v0.35.0
@@ -340,7 +341,6 @@ require (
 	github.com/AzureAD/microsoft-authentication-library-for-go v1.1.1 // indirect
 	github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.21.0 // indirect
 	github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.45.0 // indirect
-	github.com/Masterminds/semver/v3 v3.2.1 // indirect
 	github.com/Masterminds/squirrel v1.5.4 // indirect
 	github.com/PuerkitoBio/goquery v1.8.1 // indirect
 	github.com/agnivade/levenshtein v1.1.1 // indirect
diff --git a/internal/appliance/BUILD.bazel b/internal/appliance/BUILD.bazel
@@ -1,3 +1,4 @@
+load("//dev:go_defs.bzl", "go_test")
 load("@io_bazel_rules_go//go:def.bzl", "go_library")
 
 go_library(
@@ -8,6 +9,7 @@ go_library(
         "grpc.go",
         "html.go",
         "routes.go",
+        "versions.go",
     ],
     embedsrcs = [
         "web/static/img/favicon.png",
@@ -22,8 +24,12 @@ go_library(
     deps = [
         "//internal/appliance/config",
         "//internal/appliance/v1:appliance",
+        "//internal/releaseregistry",
+        "//lib/errors",
         "//lib/pointers",
         "@com_github_gorilla_mux//:mux",
+        "@com_github_life4_genesis//slices",
+        "@com_github_masterminds_semver_v3//:semver",
         "@com_github_sourcegraph_log//:log",
         "@io_k8s_api//core/v1:core",
         "@io_k8s_apimachinery//pkg/api/errors",
@@ -38,3 +44,12 @@ filegroup(
     name = "testdata",
     srcs = glob(["testdata/**"]),
 )
+
+go_test(
+    name = "appliance_test",
+    srcs = ["versions_test.go"],
+    deps = [
+        ":appliance",
+        "@com_github_stretchr_testify//require",
+    ],
+)
diff --git a/internal/appliance/appliance.go b/internal/appliance/appliance.go
@@ -13,15 +13,18 @@ import (
 	"github.com/sourcegraph/log"
 	"github.com/sourcegraph/sourcegraph/internal/appliance/config"
 	pb "github.com/sourcegraph/sourcegraph/internal/appliance/v1"
+	"github.com/sourcegraph/sourcegraph/internal/releaseregistry"
 	"github.com/sourcegraph/sourcegraph/lib/pointers"
 )
 
 type Appliance struct {
-	client      client.Client
-	namespace   string
-	status      Status
-	sourcegraph config.Sourcegraph
-	logger      log.Logger
+	client                 client.Client
+	namespace              string
+	status                 Status
+	sourcegraph            config.Sourcegraph
+	releaseRegistryClient  *releaseregistry.Client
+	latestSupportedVersion string
+	logger                 log.Logger
 
 	// Embed the UnimplementedApplianceServiceServer structs to ensure forwards compatibility (if the service is
 	// compiled against a newer version of the proto file, the server will still have default implementations of any new
@@ -42,13 +45,21 @@ func (s Status) String() string {
 	return string(s)
 }
 
-func NewAppliance(client client.Client, namespace string, logger log.Logger) *Appliance {
+func NewAppliance(
+	client client.Client,
+	relregClient *releaseregistry.Client,
+	latestSupportedVersion string,
+	namespace string,
+	logger log.Logger,
+) *Appliance {
 	return &Appliance{
-		client:      client,
-		namespace:   namespace,
-		status:      StatusSetup,
-		sourcegraph: config.Sourcegraph{},
-		logger:      logger,
+		client:                 client,
+		releaseRegistryClient:  relregClient,
+		latestSupportedVersion: latestSupportedVersion,
+		namespace:              namespace,
+		status:                 StatusSetup,
+		sourcegraph:            config.Sourcegraph{},
+		logger:                 logger,
 	}
 }
 
diff --git a/internal/appliance/html.go b/internal/appliance/html.go
@@ -2,12 +2,16 @@ package appliance
 
 import (
 	"context"
+	"fmt"
 	"html/template"
 	"net/http"
 
+	"github.com/life4/genesis/slices"
+
 	"github.com/sourcegraph/log"
 
 	"github.com/sourcegraph/sourcegraph/internal/appliance/config"
+	"github.com/sourcegraph/sourcegraph/internal/releaseregistry"
 )
 
 const (
@@ -29,11 +33,47 @@ func (a *Appliance) applianceHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 func (a *Appliance) getSetupHandler(w http.ResponseWriter, r *http.Request) {
-	err := setupTmpl.Execute(w, "")
+	versions, err := a.getVersions(r.Context())
 	if err != nil {
-		a.logger.Error("failed to execute templating", log.Error(err))
-		// Handle err
+		a.handleError(w, err, "getting versions")
+		return
+	}
+	versions, err = NMinorVersions(versions, a.latestSupportedVersion, 2)
+	if err != nil {
+		a.handleError(w, err, "filtering versions to 2 minor points")
+		return
+	}
+
+	err = setupTmpl.Execute(w, struct {
+		Versions []string
+	}{
+		Versions: versions,
+	})
+	if err != nil {
+		a.handleError(w, err, "executing template")
+		return
+	}
+}
+
+func (a *Appliance) handleError(w http.ResponseWriter, err error, msg string) {
+	a.logger.Error(msg, log.Error(err))
+
+	// TODO we should probably look twice at this and decide whether it's in
+	// line with existing standards.
+	// Don't leak details of internal errors to users - that's why we have
+	// logging above.
+	w.WriteHeader(http.StatusInternalServerError)
+	fmt.Fprintln(w, "Something went wrong - please contact support.")
+}
+
+func (a *Appliance) getVersions(ctx context.Context) ([]string, error) {
+	versions, err := a.releaseRegistryClient.ListVersions(ctx, "sourcegraph")
+	if err != nil {
+		return nil, err
 	}
+	return slices.MapFilter(versions, func(version releaseregistry.ReleaseInfo) (string, bool) {
+		return version.Version, version.Public
+	}), nil
 }
 
 func (a *Appliance) postSetupHandler(w http.ResponseWriter, r *http.Request) {
diff --git a/internal/appliance/reconciler/cadvisor.go b/internal/appliance/reconciler/cadvisor.go
@@ -20,7 +20,7 @@ import (
 
 func (r *Reconciler) reconcileCadvisor(ctx context.Context, sg *config.Sourcegraph, owner client.Object) error {
 	if err := r.reconcileCadvisorDaemonset(ctx, sg, owner); err != nil {
-		return errors.Wrap(err, "reconciling ClusterRole")
+		return errors.Wrap(err, "reconciling Daemonset")
 	}
 	if err := r.reconcileCadvisorServiceAccount(ctx, sg, owner); err != nil {
 		return errors.Wrap(err, "reconciling ServiceAccount")
diff --git a/internal/appliance/versions.go b/internal/appliance/versions.go
@@ -0,0 +1,91 @@
+package appliance
+
+import (
+	"sort"
+
+	"github.com/Masterminds/semver/v3"
+	"github.com/life4/genesis/slices"
+
+	"github.com/sourcegraph/sourcegraph/lib/errors"
+)
+
+// Return the subset of allVersions that are at most n minor revisions behind
+// latestSupportedVersion.
+func NMinorVersions(allVersions []string, latestSupportedVersion string, n uint64) ([]string, error) {
+	latestSupported, err := semver.NewVersion(latestSupportedVersion)
+	if err != nil {
+		return nil, errors.Wrap(err, "parsing latest supported version")
+	}
+
+	versionsAndErrs := slices.Map(allVersions, func(versionStr string) semverAndError {
+		version, err := semver.NewVersion(versionStr)
+		return semverAndError{semver: version, err: errors.Wrapf(err, "error parsing semver: %s", versionStr)}
+	})
+	versions := make([]*semver.Version, len(versionsAndErrs))
+	for i, versionAndErr := range versionsAndErrs {
+		if versionAndErr.err != nil {
+			return nil, err
+		}
+		versions[i] = versionAndErr.semver
+	}
+	sort.Sort(semver.Collection(versions))
+
+	var nminor []*semver.Version
+	for _, version := range versions {
+		if latestSupported.Major() != version.Major() {
+			continue
+		}
+		if version.GreaterThan(latestSupported) {
+			continue
+		}
+		if latestSupported.Minor()-version.Minor() > n {
+			continue
+		}
+
+		nminor = append(nminor, version)
+	}
+
+	// If we have not collected n minor versions, we tolerate one major version
+	// transition and search for the remaining (n - whatever) minor version
+	// matches there.
+	if len(nminor) == 0 {
+		return nil, errors.Newf("found no versions within %d minor revisions of %s", n, latestSupportedVersion)
+	}
+	if latestSupported.Minor()-nminor[0].Minor() < n {
+		for _, version := range versions {
+			if latestSupported.Major()-version.Major() == 1 {
+				highestMinor := highestMinorInMajorSeries(versions, version.Major())
+				lowestToleratedMinor := highestMinor - (n - 1)
+				if version.Minor() >= lowestToleratedMinor {
+					nminor = append(nminor, version)
+				}
+			}
+		}
+
+		sort.Sort(semver.Collection(nminor))
+	}
+
+	sort.Sort(sort.Reverse(semver.Collection(nminor)))
+	return slices.Map(nminor, func(semver *semver.Version) string { return semver.String() }), nil
+}
+
+func highestMinorInMajorSeries(versions []*semver.Version, major uint64) uint64 {
+	// iterate backwards to start with the highest numbers
+	for i := len(versions) - 1; i >= 0; i-- {
+		if major == versions[i].Major() {
+			return versions[i].Minor()
+		}
+	}
+
+	// We shouldn't need to return an error here, this is in-practice
+	// unreachable (famous last words).
+	// We only call this function with values of major that are present in the
+	// versions array, so there will always be a match for the if-statement in
+	// the loop above.
+	return 0
+}
+
+type semverAndError struct {
+	semver *semver.Version
+	err    error
+}
diff --git a/internal/appliance/versions_test.go b/internal/appliance/versions_test.go
diff --git a/internal/appliance/web/template/setup.gohtml b/internal/appliance/web/template/setup.gohtml
diff --git a/internal/releaseregistry/client.go b/internal/releaseregistry/client.go

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ import (`
`20`	`20`
`21`	`21`	`func (r Reconciler) reconcileCadvisor(ctx context.Context, sg config.Sourcegraph, owner client.Object) error {`
`22`	`22`	`if err := r.reconcileCadvisorDaemonset(ctx, sg, owner); err != nil {`
`23`		`- return errors.Wrap(err, "reconciling ClusterRole")`
	`23`	`+ return errors.Wrap(err, "reconciling Daemonset")`
`24`	`24`	`}`
`25`	`25`	`if err := r.reconcileCadvisorServiceAccount(ctx, sg, owner); err != nil {`
`26`	`26`	`return errors.Wrap(err, "reconciling ServiceAccount")`