From 2b352ba25ed0adbd4d559984b70c879d4e9c9b81 Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Mon, 17 Feb 2025 13:59:37 +0100 Subject: [PATCH] versionsapi: canonicalize ref in version constructors Co-authored-by: 3u13r Signed-off-by: Paul Meyer --- internal/api/versionsapi/cli/add.go | 95 +++++------------ internal/api/versionsapi/version.go | 9 +- internal/api/versionsapi/version_test.go | 125 +++++++++++++++++++++++ 3 files changed, 155 insertions(+), 74 deletions(-) diff --git a/internal/api/versionsapi/cli/add.go b/internal/api/versionsapi/cli/add.go index f1a6fc4fd..c28505c79 100644 --- a/internal/api/versionsapi/cli/add.go +++ b/internal/api/versionsapi/cli/add.go @@ -16,7 +16,6 @@ import ( "github.com/edgelesssys/constellation/v2/internal/api/versionsapi" "github.com/edgelesssys/constellation/v2/internal/logger" "github.com/spf13/cobra" - "golang.org/x/mod/semver" ) func newAddCmd() *cobra.Command { @@ -53,19 +52,8 @@ func runAdd(cmd *cobra.Command, _ []string) (retErr error) { return err } log := logger.NewTextLogger(flags.logLevel) - log.Debug("Using flags", "dryRun", flags.dryRun, "kind", flags.kind, "latest", flags.latest, "ref", flags.ref, - "release", flags.release, "stream", flags.stream, "version", flags.version) - - log.Debug("Validating flags") - if err := flags.validate(log); err != nil { - return err - } - - log.Debug("Creating version struct") - ver, err := versionsapi.NewVersion(flags.ref, flags.stream, flags.version, flags.kind) - if err != nil { - return fmt.Errorf("creating version: %w", err) - } + log.Debug("Using flags", "dryRun", flags.dryRun, "kind", flags.version.Kind(), "latest", flags.latest, "ref", flags.version.Ref(), + "stream", flags.version.Stream(), "version", flags.version.Version()) log.Debug("Creating versions API client") client, clientClose, err := versionsapi.NewClient(cmd.Context(), flags.region, flags.bucket, flags.distributionID, flags.dryRun, log) @@ -80,27 +68,27 @@ func runAdd(cmd *cobra.Command, _ []string) (retErr error) { }() log.Info("Adding version") - if err := ensureVersion(cmd.Context(), client, flags.kind, ver, versionsapi.GranularityMajor, log); err != nil { + if err := ensureVersion(cmd.Context(), client, flags.version, versionsapi.GranularityMajor, log); err != nil { return err } - if err := ensureVersion(cmd.Context(), client, flags.kind, ver, versionsapi.GranularityMinor, log); err != nil { + if err := ensureVersion(cmd.Context(), client, flags.version, versionsapi.GranularityMinor, log); err != nil { return err } if flags.latest { - if err := updateLatest(cmd.Context(), client, flags.kind, ver, log); err != nil { + if err := updateLatest(cmd.Context(), client, flags.version, log); err != nil { return fmt.Errorf("setting latest version: %w", err) } } - log.Info(fmt.Sprintf("List major->minor URL: %s", ver.ListURL(versionsapi.GranularityMajor))) - log.Info(fmt.Sprintf("List minor->patch URL: %s", ver.ListURL(versionsapi.GranularityMinor))) + log.Info(fmt.Sprintf("List major->minor URL: %s", flags.version.ListURL(versionsapi.GranularityMajor))) + log.Info(fmt.Sprintf("List minor->patch URL: %s", flags.version.ListURL(versionsapi.GranularityMinor))) return nil } -func ensureVersion(ctx context.Context, client *versionsapi.Client, kind versionsapi.VersionKind, ver versionsapi.Version, gran versionsapi.Granularity, +func ensureVersion(ctx context.Context, client *versionsapi.Client, ver versionsapi.Version, gran versionsapi.Granularity, log *slog.Logger, ) error { verListReq := versionsapi.List{ @@ -108,7 +96,7 @@ func ensureVersion(ctx context.Context, client *versionsapi.Client, kind version Stream: ver.Stream(), Granularity: gran, Base: ver.WithGranularity(gran), - Kind: kind, + Kind: ver.Kind(), } verList, err := client.FetchVersionList(ctx, verListReq) var notFoundErr *apiclient.NotFoundError @@ -140,11 +128,11 @@ func ensureVersion(ctx context.Context, client *versionsapi.Client, kind version return nil } -func updateLatest(ctx context.Context, client *versionsapi.Client, kind versionsapi.VersionKind, ver versionsapi.Version, log *slog.Logger) error { +func updateLatest(ctx context.Context, client *versionsapi.Client, ver versionsapi.Version, log *slog.Logger) error { latest := versionsapi.Latest{ Ref: ver.Ref(), Stream: ver.Stream(), - Kind: kind, + Kind: ver.Kind(), } latest, err := client.FetchVersionLatest(ctx, latest) var notFoundErr *apiclient.NotFoundError @@ -164,7 +152,7 @@ func updateLatest(ctx context.Context, client *versionsapi.Client, kind versions Ref: ver.Ref(), Stream: ver.Stream(), Version: ver.Version(), - Kind: kind, + Kind: ver.Kind(), } if err := client.UpdateVersionLatest(ctx, latest); err != nil { return fmt.Errorf("updating latest version: %w", err) @@ -174,60 +162,20 @@ func updateLatest(ctx context.Context, client *versionsapi.Client, kind versions } type addFlags struct { - version string - stream string - ref string - release bool + version versionsapi.Version latest bool dryRun bool region string bucket string distributionID string - kind versionsapi.VersionKind logLevel slog.Level } -func (f *addFlags) validate(log *slog.Logger) error { - if !semver.IsValid(f.version) { - return fmt.Errorf("version %q is not a valid semantic version", f.version) - } - if semver.Canonical(f.version) != f.version { - return fmt.Errorf("version %q is not a canonical semantic version", f.version) - } - - if f.ref == "" && !f.release { - return fmt.Errorf("either --ref or --release must be set") - } - - if f.kind == versionsapi.VersionKindUnknown { - return fmt.Errorf("unknown version kind %q", f.kind) - } - - if f.release { - log.Debug(fmt.Sprintf("Setting ref to %q, as release flag is set", versionsapi.ReleaseRef)) - f.ref = versionsapi.ReleaseRef - } else { - log.Debug("Setting latest to true, as release flag is not set") - f.latest = true // always set latest for non-release versions - } - - if err := versionsapi.ValidateRef(f.ref); err != nil { - return fmt.Errorf("invalid ref %w", err) - } - - if err := versionsapi.ValidateStream(f.ref, f.stream); err != nil { - return fmt.Errorf("invalid stream %w", err) - } - - return nil -} - func parseAddFlags(cmd *cobra.Command) (addFlags, error) { ref, err := cmd.Flags().GetString("ref") if err != nil { return addFlags{}, err } - ref = versionsapi.CanonicalizeRef(ref) stream, err := cmd.Flags().GetString("stream") if err != nil { return addFlags{}, err @@ -274,17 +222,24 @@ func parseAddFlags(cmd *cobra.Command) (addFlags, error) { return addFlags{}, err } + if release { + ref = versionsapi.ReleaseRef + } else { + latest = true // always set latest for non-release versions + } + + ver, err := versionsapi.NewVersion(ref, stream, version, kind) + if err != nil { + return addFlags{}, fmt.Errorf("creating version: %w", err) + } + return addFlags{ - version: version, - stream: stream, - ref: versionsapi.CanonicalizeRef(ref), - release: release, + version: ver, latest: latest, dryRun: dryRun, region: region, bucket: bucket, distributionID: distributionID, logLevel: logLevel, - kind: kind, }, nil } diff --git a/internal/api/versionsapi/version.go b/internal/api/versionsapi/version.go index 12d1e8100..180c27e5f 100644 --- a/internal/api/versionsapi/version.go +++ b/internal/api/versionsapi/version.go @@ -41,7 +41,7 @@ type Version struct { // NewVersion creates a new Version object and validates it. func NewVersion(ref, stream, version string, kind VersionKind) (Version, error) { ver := Version{ - ref: ref, + ref: CanonicalizeRef(ref), stream: stream, version: version, kind: kind, @@ -62,7 +62,7 @@ func NewVersionFromShortPath(shortPath string, kind VersionKind) (Version, error } ver := Version{ - ref: ref, + ref: ref, // Canonicalized by parseShortPath. stream: stream, version: version, kind: kind, @@ -331,7 +331,7 @@ func CanonicalizeRef(ref string) string { canRef := notAZ09Regexp.ReplaceAllString(ref, "-") if canRef == ReleaseRef { - return "" // No ref should be cannonicalized to the release ref. + return "" // No ref should be canonicalized to the release ref. } return canRef @@ -401,7 +401,7 @@ func MeasurementURL(version Version) (measurementURL, signatureURL *url.URL, err } var ( - shortPathRegex = regexp.MustCompile(`^ref/([a-zA-Z0-9-]+)/stream/([a-zA-Z0-9-]+)/([a-zA-Z0-9.-]+)$`) + shortPathRegex = regexp.MustCompile(`^ref/([^/]+)/stream/([a-zA-Z0-9-]+)/([a-zA-Z0-9.-]+)$`) shortPathReleaseRegex = regexp.MustCompile(`^stream/([a-zA-Z0-9-]+)/([a-zA-Z0-9.-]+)$`) ) @@ -422,6 +422,7 @@ func parseShortPath(shortPath string) (ref, stream, version string, err error) { if shortPathRegex.MatchString(shortPath) { matches := shortPathRegex.FindStringSubmatch(shortPath) ref := matches[1] + ref = CanonicalizeRef(ref) if err := ValidateRef(ref); err != nil { return "", "", "", err } diff --git a/internal/api/versionsapi/version_test.go b/internal/api/versionsapi/version_test.go index 25f0f8ce0..66e48d2b9 100644 --- a/internal/api/versionsapi/version_test.go +++ b/internal/api/versionsapi/version_test.go @@ -16,6 +16,111 @@ import ( "github.com/edgelesssys/constellation/v2/internal/constants" ) +func TestNewVersion(t *testing.T) { + testCases := map[string]struct { + ref string + stream string + version string + kind VersionKind + wantVer Version + wantErr bool + }{ + "stable release image": { + ref: ReleaseRef, + stream: "stable", + version: "v9.9.9", + kind: VersionKindImage, + wantVer: Version{ + ref: ReleaseRef, + stream: "stable", + version: "v9.9.9", + kind: VersionKindImage, + }, + }, + "release debug image": { + ref: ReleaseRef, + stream: "debug", + version: "v9.9.9", + kind: VersionKindImage, + wantVer: Version{ + ref: ReleaseRef, + stream: "debug", + version: "v9.9.9", + kind: VersionKindImage, + }, + }, + "stable release cli": { + ref: ReleaseRef, + stream: "stable", + version: "v9.9.9", + kind: VersionKindCLI, + wantVer: Version{ + ref: ReleaseRef, + stream: "stable", + version: "v9.9.9", + kind: VersionKindCLI, + }, + }, + "release debug cli": { + ref: ReleaseRef, + stream: "debug", + version: "v9.9.9", + kind: VersionKindCLI, + wantVer: Version{ + ref: ReleaseRef, + stream: "debug", + version: "v9.9.9", + kind: VersionKindCLI, + }, + }, + "unknown kind": { + ref: ReleaseRef, + stream: "debug", + version: "v9.9.9", + kind: VersionKindUnknown, + wantErr: true, + }, + "non-release ref as input": { + ref: "working-branch", + stream: "debug", + version: "v9.9.9", + kind: VersionKindImage, + wantVer: Version{ + ref: "working-branch", + stream: "debug", + version: "v9.9.9", + kind: VersionKindImage, + }, + }, + "non-canonical ref as input": { + ref: "testing-1.23", + stream: "debug", + version: "v9.9.9", + kind: VersionKindImage, + wantVer: Version{ + ref: "testing-1-23", + stream: "debug", + version: "v9.9.9", + kind: VersionKindImage, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + + ver, err := NewVersion(tc.ref, tc.stream, tc.version, tc.kind) + if tc.wantErr { + assert.Error(err) + return + } + assert.NoError(err) + assert.Equal(tc.wantVer, ver) + }) + } +} + func TestNewVersionFromShortPath(t *testing.T) { testCases := map[string]struct { path string @@ -78,6 +183,26 @@ func TestNewVersionFromShortPath(t *testing.T) { kind: VersionKindCLI, wantErr: true, }, + "non-release ref as input": { + path: "ref/working-branch/stream/debug/v9.9.9", + kind: VersionKindImage, + wantVer: Version{ + ref: "working-branch", + stream: "debug", + version: "v9.9.9", + kind: VersionKindImage, + }, + }, + "non-canonical ref as input": { + path: "ref/testing-1.23/stream/debug/v9.9.9", + kind: VersionKindImage, + wantVer: Version{ + ref: "testing-1-23", + stream: "debug", + version: "v9.9.9", + kind: VersionKindImage, + }, + }, } for name, tc := range testCases {