feat(init): space init fits for v1.7.0 with registry change and diffe…

…rent pre-reqs

Signed-off-by: Christopher Haar <[email protected]>

cmd/up/space/flags.go

@@ -4,22 +4,15 @@
 package space
 
 import (
-	"encoding/json"
-	"io"
 	"net/url"
 	"os"
 
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
 
 	"github.com/upbound/up/internal/input"
+	"github.com/upbound/up/internal/upbound"
 )
 
-// tokenData holding the parsed token data
-type tokenData struct {
-	AccessID string `json:"accessId"`
-	Token    string `json:"token"`
-}
-
 type registryFlags struct {
 	Repository *url.URL `hidden:"" name:"registry-repository" env:"UPBOUND_REGISTRY" default:"xpkg.upbound.io/spaces-artifacts" help:"Set registry for where to pull OCI artifacts from. This is an OCI registry reference, i.e. a URL without the scheme or protocol prefix."`
 	Endpoint   *url.URL `hidden:"" name:"registry-endpoint" env:"UPBOUND_REGISTRY_ENDPOINT" default:"https://xpkg.upbound.io" help:"Set registry endpoint, including scheme, for authentication."`
@@ -28,7 +21,7 @@ type registryFlags struct {
 type authorizedRegistryFlags struct {
 	registryFlags
 
-	TokenFile *os.File `name:"token-file" help:"File containing authentication token."`
+	TokenFile *os.File `name:"token-file" help:"File containing authentication token. Expecting a JSON file with \"accessId\" and \"token\" keys."`
 	Username  string   `hidden:"" name:"registry-username" help:"Set the registry username."`
 	Password  string   `hidden:"" name:"registry-password" help:"Set the registry password."`
 }
@@ -58,19 +51,11 @@ func (p *authorizedRegistryFlags) AfterApply() error {
 		return nil
 	}
 
-	b, err := io.ReadAll(p.TokenFile)
-	defer p.TokenFile.Close() // nolint:errcheck
+	tf, err := upbound.TokenFromPath(p.TokenFile.Name())
 	if err != nil {
-		return errors.Wrap(err, errReadTokenFile)
+		return err
 	}
-
-	var tokenData tokenData
-	if err := json.Unmarshal(b, &tokenData); err != nil {
-		return errors.Wrap(err, errReadTokenFileData)
-	}
-
-	p.Username = tokenData.AccessID
-	p.Password = tokenData.Token
+	p.Username, p.Password = tf.AccessID, tf.Token
 
 	return nil
 }

cmd/up/space/init.go

@@ -386,9 +386,9 @@ func (c *initCmd) deploySpace(ctx context.Context, params map[string]any) error
 	hcSpinner, _ := upterm.CheckmarkSuccessSpinner.Start(upterm.StepCounter("Starting Space Components", 3, 3))
 
 	version, _ := semver.NewVersion(c.Version)
-	constraint, _ := semver.NewConstraint("< v1.7.0-0")
+	requiresUXP, _ := semver.NewConstraint("< v1.7.0-0")
 
-	if constraint.Check(version) {
+	if requiresUXP.Check(version) {
 		errC, err := kube.DynamicWatch(ctx, c.dClient.Resource(hostclusterGVR), &watcherTimeout, func(u *unstructured.Unstructured) (bool, error) {
 			up := resources.HostCluster{Unstructured: *u}
 			if resource.IsConditionTrue(up.GetCondition(xpv1.TypeReady)) {

cmd/up/space/prerequisites/manager.go

@@ -64,13 +64,13 @@ func New(config *rest.Config, defs *defaults.CloudConfig, features *feature.Flag
 		return nil, errors.New("invalid version format: " + err.Error())
 	}
 
-	constraint, err := semver.NewConstraint("< v1.7.0-0")
+	requiresUXP, err := semver.NewConstraint("< v1.7.0-0")
 	if err != nil {
 		return nil, errors.New("invalid version constraint: " + err.Error())
 	}
 
 	// Check if the version satisfies the constraint
-	if constraint.Check(version) {
+	if requiresUXP.Check(version) {
 		uxp, err := uxp.New(config)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create UXP prerequisite")

cmd/up/space/prerequisites/manager_test.go

@@ -20,7 +20,6 @@ import (
 	"k8s.io/client-go/rest"
 
 	"github.com/crossplane/crossplane-runtime/pkg/feature"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/upbound/up/cmd/up/space/defaults"
@@ -35,82 +34,109 @@ import (
 
 // Mock implementations or test helpers would be needed for uxp, kubernetes, helm, certmanager, ingressnginx, and opentelemetrycollector.
 func TestNew(t *testing.T) {
-	tests := []struct {
-		name            string
-		config          *rest.Config
-		defs            *defaults.CloudConfig
-		setupFeatures   func() *feature.Flags
-		versionStr      string
+	type args struct {
+		config        *rest.Config
+		defs          *defaults.CloudConfig
+		setupFeatures func() *feature.Flags
+		versionStr    string
+	}
+
+	type want struct {
 		expectError     bool
 		expectedErrMsg  string
-		expectedPrereqs []string // Expected types or identifiers of prerequisites
+		expectedPrereqs []string
+	}
+
+	cases := map[string]struct {
+		reason string
+		args   args
+		want   want
 	}{
-		{
-			name:            "Invalid version format",
-			config:          &rest.Config{},
-			defs:            &defaults.CloudConfig{},
-			setupFeatures:   func() *feature.Flags { return &feature.Flags{} },
-			versionStr:      "invalid-version",
-			expectError:     true,
-			expectedErrMsg:  "invalid version format",
-			expectedPrereqs: nil, // No prerequisites expected
+		"InvalidVersionFormat": {
+			reason: "Testing invalid version format should return an error.",
+			args: args{
+				config:        &rest.Config{},
+				defs:          &defaults.CloudConfig{},
+				setupFeatures: func() *feature.Flags { return &feature.Flags{} },
+				versionStr:    "invalid-version",
+			},
+			want: want{
+				expectError:    true,
+				expectedErrMsg: "invalid version format",
+			},
 		},
-		{
-			name:   "Version less than 1.7.0 with prerequisites",
-			config: &rest.Config{},
-			defs:   &defaults.CloudConfig{PublicIngress: false},
-			setupFeatures: func() *feature.Flags {
-				return &feature.Flags{} // No features enabled
+		"VersionLessThan170WithPrerequisites": {
+			reason: "Testing version less than 1.7.0 should have specific prerequisites.",
+			args: args{
+				config: &rest.Config{},
+				defs:   &defaults.CloudConfig{PublicIngress: false},
+				setupFeatures: func() *feature.Flags {
+					return &feature.Flags{}
+				},
+				versionStr: "v1.6.0",
+			},
+			want: want{
+				expectError:     false,
+				expectedPrereqs: []string{"uxp", "kubernetes", "helm", "certmanager", "ingressnginx"},
 			},
-			versionStr:      "v1.6.0",
-			expectError:     false,
-			expectedPrereqs: []string{"uxp", "kubernetes", "helm", "certmanager", "ingressnginx"},
 		},
-		{
-			name:   "Version greater than or equal to 1.7.0 without certain prerequisites",
-			config: &rest.Config{},
-			defs:   &defaults.CloudConfig{PublicIngress: true},
-			setupFeatures: func() *feature.Flags {
-				return &feature.Flags{} // No features enabled
+		"VersionGreaterThanOrEqualTo170WithoutCertainPrerequisites": {
+			reason: "Testing version >= 1.7.0 should have specific prerequisites.",
+			args: args{
+				config: &rest.Config{},
+				defs:   &defaults.CloudConfig{PublicIngress: true},
+				setupFeatures: func() *feature.Flags {
+					return &feature.Flags{}
+				},
+				versionStr: "v1.8.0",
+			},
+			want: want{
+				expectError:     false,
+				expectedPrereqs: []string{"certmanager", "ingressnginx"},
 			},
-			versionStr:      "v1.8.0",
-			expectError:     false,
-			expectedPrereqs: []string{"certmanager", "ingressnginx"},
 		},
-		{
-			name:   "Version is rc of 1.7.0 without certain prerequisites",
-			config: &rest.Config{},
-			defs:   &defaults.CloudConfig{PublicIngress: true},
-			setupFeatures: func() *feature.Flags {
-				return &feature.Flags{} // No features enabled
+		"VersionIsRCof170WithoutCertainPrerequisites": {
+			reason: "Testing a release candidate version of 1.7.0 should have specific prerequisites.",
+			args: args{
+				config: &rest.Config{},
+				defs:   &defaults.CloudConfig{PublicIngress: true},
+				setupFeatures: func() *feature.Flags {
+					return &feature.Flags{}
+				},
+				versionStr: "v1.7.0-rc.0.221.gd1b9198d",
+			},
+			want: want{
+				expectError:     false,
+				expectedPrereqs: []string{"certmanager", "ingressnginx"},
 			},
-			versionStr:      "v1.7.0-rc.0.221.gd1b9198d",
-			expectError:     false,
-			expectedPrereqs: []string{"certmanager", "ingressnginx"},
 		},
-		{
-			name:   "Feature enabled with alpha shared telemetry",
-			config: &rest.Config{},
-			defs:   &defaults.CloudConfig{},
-			setupFeatures: func() *feature.Flags {
-				flags := &feature.Flags{}
-				flags.Enable(spacefeature.EnableAlphaSharedTelemetry) // Enable the alpha shared telemetry feature
-				return flags
+		"FeatureEnabledWithAlphaSharedTelemetry": {
+			reason: "Testing feature flag enabling alpha shared telemetry should add the opentelemetrycollector prerequisite.",
+			args: args{
+				config: &rest.Config{},
+				defs:   &defaults.CloudConfig{},
+				setupFeatures: func() *feature.Flags {
+					flags := &feature.Flags{}
+					flags.Enable(spacefeature.EnableAlphaSharedTelemetry)
+					return flags
+				},
+				versionStr: "v1.8.0",
+			},
+			want: want{
+				expectError:     false,
+				expectedPrereqs: []string{"certmanager", "ingressnginx", "opentelemetrycollector"},
 			},
-			versionStr:      "v1.8.0",
-			expectError:     false,
-			expectedPrereqs: []string{"certmanager", "ingressnginx", "opentelemetrycollector"},
 		},
 	}
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			features := tt.setupFeatures() // Initialize feature flags using setup function
-			manager, err := New(tt.config, tt.defs, features, tt.versionStr)
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			features := tc.args.setupFeatures() // Initialize feature flags using setup function
+			manager, err := New(tc.args.config, tc.args.defs, features, tc.args.versionStr)
 
-			if tt.expectError {
+			if tc.want.expectError {
 				require.Error(t, err)
-				require.Contains(t, err.Error(), tt.expectedErrMsg)
+				require.Contains(t, err.Error(), tc.want.expectedErrMsg)
 			} else {
 				require.NoError(t, err)
 				require.NotNil(t, manager)
@@ -136,7 +162,7 @@ func TestNew(t *testing.T) {
 					}
 				}
 
-				assert.ElementsMatch(t, tt.expectedPrereqs, prereqTypes)
+				require.ElementsMatch(t, tc.want.expectedPrereqs, prereqTypes)
 			}
 		})
 	}