From dba7f33d5d92c64d0e2ce0be030b31e414f1ed49 Mon Sep 17 00:00:00 2001 From: Christopher Haar Date: Mon, 26 Aug 2024 13:32:14 +0200 Subject: [PATCH] feat(init): space init fits for v1.7.0 with registry change and different pre-reqs Signed-off-by: Christopher Haar --- cmd/up/space/flags.go | 25 +--- cmd/up/space/init.go | 4 +- cmd/up/space/prerequisites/manager.go | 4 +- cmd/up/space/prerequisites/manager_test.go | 150 ++++++++++++--------- 4 files changed, 97 insertions(+), 86 deletions(-) diff --git a/cmd/up/space/flags.go b/cmd/up/space/flags.go index 19c71966..c1239ce7 100644 --- a/cmd/up/space/flags.go +++ b/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 } diff --git a/cmd/up/space/init.go b/cmd/up/space/init.go index 37a8208d..41bc22f5 100644 --- a/cmd/up/space/init.go +++ b/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)) { diff --git a/cmd/up/space/prerequisites/manager.go b/cmd/up/space/prerequisites/manager.go index a1bd7698..b7d81716 100644 --- a/cmd/up/space/prerequisites/manager.go +++ b/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") diff --git a/cmd/up/space/prerequisites/manager_test.go b/cmd/up/space/prerequisites/manager_test.go index 14dd1ca6..80205053 100644 --- a/cmd/up/space/prerequisites/manager_test.go +++ b/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) } }) }