otto feedback

dev-docs/workflows/attestationconfigapi.md

@@ -8,10 +8,10 @@ This estimate might make manual intervention necessary when a global rollout did
 
 ### Manually delete a version
 ```
-COSIGN_PASSWORD=$PWD COSIGN_PRIVATE_KEY="$(cat $PATH_TO_KEY)" AWS_ACCESS_KEY_ID=$ID AWS_ACCESS_KEY=$KEY bazel run //internal/api/attestationconfigapi/cli delete -- --version 2023-09-02-12-52
+COSIGN_PASSWORD=$CPW COSIGN_PRIVATE_KEY="$(cat $PATH_TO_KEY)" AWS_ACCESS_KEY_ID=$ID AWS_ACCESS_KEY=$KEY bazel run //internal/api/attestationconfigapi/cli delete -- --version 2023-09-02-12-52
 ```
 
 ### Manually upload a version
 ```
-COSIGN_PASSWORD=$PWD COSIGN_PRIVATE_KEY="$(cat $PATH_TO_KEY)" AWS_ACCESS_KEY_ID=$ID AWS_ACCESS_KEY=$KEY bazel run //internal/api/attestationconfigapi/cli -- --force --version 2023-09-02-12-52  --maa-claims-path "${path}"
+COSIGN_PASSWORD=$CPW COSIGN_PRIVATE_KEY="$(cat $PATH_TO_KEY)" AWS_ACCESS_KEY_ID=$ID AWS_ACCESS_KEY=$KEY bazel run //internal/api/attestationconfigapi/cli -- --force --version 2023-09-02-12-52  --maa-claims-path "${path}"
 ```

internal/api/attestationconfigapi/cli/main.go

@@ -55,10 +55,10 @@ func main() {
 // newRootCmd creates the root command.
 func newRootCmd() *cobra.Command {
 	rootCmd := &cobra.Command{
-		Use:   "COSIGN_PASSWORD=$CPWD COSIGN_PRIVATE_KEY=$CKEY upload --version-file $FILE",
+		Use:   "COSIGN_PASSWORD=$CPW COSIGN_PRIVATE_KEY=$CKEY upload --version-file $FILE",
 		Short: "Upload a set of versions specific to the azure-sev-snp attestation variant to the config api.",
 
-		Long: fmt.Sprintf("The reporter uploads an observed version number specific to the azure-sev-snp attestation variant to the reporter cache. The reporter then deduces a latest version value from the cache and updates the config api if necessary. "+
+		Long: fmt.Sprintf("The CLI uploads an observed version number specific to the azure-sev-snp attestation variant to a cache directory. The CLI then determines the lowest version within the cache-window present in the cache and writes that value to the config api if necessary. "+
 			"Please authenticate with AWS through your preferred method (e.g. environment variables, CLI)"+
 			"to be able to upload to S3. Set the %s and %s environment variables to authenticate with cosign.",
 			envCosignPrivateKey, envCosignPwd,
@@ -69,7 +69,7 @@ func newRootCmd() *cobra.Command {
 	rootCmd.Flags().StringP("maa-claims-path", "t", "", "File path to a json file containing the MAA claims.")
 	rootCmd.Flags().StringP("upload-date", "d", "", "upload a version with this date as version name.")
 	rootCmd.Flags().BoolP("force", "f", false, "Use force to manually push a new latest version."+
-		" The version gets reported in the cache but the version selection logic is skipped.")
+		" The version gets saved to the cache but the version selection logic is skipped.")
 	rootCmd.Flags().IntP("cache-window-size", "s", 0, "Number of versions to be considered for the latest version.")
 	rootCmd.PersistentFlags().StringP("region", "r", awsRegion, "region of the targeted bucket.")
 	rootCmd.PersistentFlags().StringP("bucket", "b", awsBucket, "bucket targeted by all operations.")
@@ -129,13 +129,13 @@ func runCmd(cmd *cobra.Command, _ []string) (retErr error) {
 		}
 	}()
 	if flags.cacheWindowSize != 0 {
-		client.SetCacheVersionSize(flags.cacheWindowSize)
+		client.SetCacheWindowSize(flags.cacheWindowSize)
 	}
 
 	if err != nil {
 		return fmt.Errorf("creating client: %w", err)
 	}
-	if err := client.UpdateLatestAzureSEVSNPVersion(ctx, inputVersion, latestAPIVersion, flags.uploadDate, flags.force); err != nil {
+	if err := client.UploadAzureSEVSNPVersionLatest(ctx, inputVersion, latestAPIVersion, flags.uploadDate, flags.force); err != nil {
 		return fmt.Errorf("updating latest version: %w", err)
 	}
 	return nil

internal/api/attestationconfigapi/reporter.go

@@ -33,66 +33,67 @@ const versionWindowSize = 15
 
 var reportVersionDir = path.Join(attestationURLPath, variant.AzureSEVSNP{}.String(), cachedVersionsSubDir)
 
-// UpdateLatestAzureSEVSNPVersion reports the given version, checks the reported version values
-// and updates the latest version of the Azure SEVSNP in the API if there is an update .
-// force can be used to force an update of the latest API version regardless of the cache reporting.
-func (c Client) UpdateLatestAzureSEVSNPVersion(ctx context.Context, inputVersion,
+// UploadAzureSEVSNPVersionLatest saves the given version to the cache, determines the smallest
+// TCB version in the cache among the last cacheWindowSize versions and updates
+// the latest version in the API if there is an update.
+// force can be used to bypass the validation logic against the cached versions.
+func (c Client) UploadAzureSEVSNPVersionLatest(ctx context.Context, inputVersion,
 	latestAPIVersion AzureSEVSNPVersion, now time.Time, force bool,
 ) error {
-	if err := c.reportAzureSEVSNPVersion(ctx, inputVersion, now); err != nil {
+	if err := c.cacheAzureSEVSNPVersion(ctx, inputVersion, now); err != nil {
 		return fmt.Errorf("reporting version: %w", err)
 	}
 	if force {
-		if err := c.uploadAzureSEVSNPVersion(ctx, inputVersion, now); err != nil {
-			return fmt.Errorf("force uploading version: %w", err)
-		}
-		return nil
+		return c.uploadAzureSEVSNPVersion(ctx, inputVersion, now)
 	}
-	versionDates, err := c.listReportedVersions(ctx)
+	versionDates, err := c.listCachedVersions(ctx)
 	if err != nil {
 		return fmt.Errorf("list reported versions: %w", err)
 	}
 	if len(versionDates) < c.cacheWindowSize {
-		c.s3Client.Logger.Infof("Skipping version update since found only %d out of expected reported versions.", len(versionDates), c.cacheWindowSize)
+		c.s3Client.Logger.Warnf("Skipping version update, found %d, expected %d reported versions.", len(versionDates), c.cacheWindowSize)
 		return nil
 	}
 	minVersion, minDate, err := c.findMinVersion(ctx, versionDates)
 	if err != nil {
 		return fmt.Errorf("get minimal version: %w", err)
 	}
-	c.s3Client.Logger.Infof("Found minimal version: %+v with date: %s", *minVersion, minDate)
-	shouldUpdateAPI, err := isInputNewerThanOtherVersion(*minVersion, latestAPIVersion)
-	if err == nil && shouldUpdateAPI {
-		c.s3Client.Logger.Infof("Input version: %+v is newer than latest API version: %+v", *minVersion, latestAPIVersion)
-		t, err := time.Parse(VersionFormat, minDate)
-		if err != nil {
-			return fmt.Errorf("parsing date: %w", err)
-		}
-		if err := c.uploadAzureSEVSNPVersion(ctx, *minVersion, t); err != nil {
-			return fmt.Errorf("uploading version: %w", err)
-		}
-		c.s3Client.Logger.Infof("Successfully uploaded new Azure SEV-SNP version: %+v", *minVersion)
+	c.s3Client.Logger.Infof("Found minimal version: %+v with date: %s", minVersion, minDate)
+	shouldUpdateAPI, err := isInputNewerThanOtherVersion(minVersion, latestAPIVersion)
+	if err != nil {
+		return fmt.Errorf("comparing new and other versions: %w", err)
+	}
+	if !shouldUpdateAPI {
+		c.s3Client.Logger.Infof("Input version: %+v is not newer than latest API version: %+v", minVersion, latestAPIVersion)
 		return nil
 	}
-	c.s3Client.Logger.Infof("Input version: %+v is not newer than latest API version: %+v", *minVersion, latestAPIVersion)
+	c.s3Client.Logger.Infof("Input version: %+v is newer than latest API version: %+v", minVersion, latestAPIVersion)
+	t, err := time.Parse(VersionFormat, minDate)
+	if err != nil {
+		return fmt.Errorf("parsing date: %w", err)
+	}
+	if err := c.uploadAzureSEVSNPVersion(ctx, minVersion, t); err != nil {
+		return fmt.Errorf("uploading version: %w", err)
+	}
+	c.s3Client.Logger.Infof("Successfully uploaded new Azure SEV-SNP version: %+v", minVersion)
 	return nil
 }
 
-// SetCacheVersionSize sets a custom number of versions to be considered for the latest version.
-func (c *Client) SetCacheVersionSize(size int) {
+// SetCacheWindowSize sets a custom number of versions to be considered for the latest version.
+func (c *Client) SetCacheWindowSize(size int) {
 	c.cacheWindowSize = size
 }
 
-// reportAzureSEVSNPVersion uploads the latest observed version numbers of the Azure SEVSNP. This version is used to later report the latest version numbers to the API.
-func (c Client) reportAzureSEVSNPVersion(ctx context.Context, version AzureSEVSNPVersion, date time.Time) error {
+// cacheAzureSEVSNPVersion uploads the latest observed version numbers of the Azure SEVSNP. This version is used to later report the latest version numbers to the API.
+func (c Client) cacheAzureSEVSNPVersion(ctx context.Context, version AzureSEVSNPVersion, date time.Time) error {
 	dateStr := date.Format(VersionFormat) + ".json"
 	res := putCmdWithoutSigning{
 		apiObject: reportedAzureSEVSNPVersionAPI{Version: dateStr, AzureSEVSNPVersion: version},
 	}
 	return res.Execute(ctx, c.s3Client)
 }
 
-func (c Client) listReportedVersions(ctx context.Context) ([]string, error) {
+func (c Client) listCachedVersions(ctx context.Context) ([]string, error) {
 	list, err := c.s3Client.ListObjectsV2(ctx, &s3.ListObjectsV2Input{
 		Bucket: aws.String(c.bucketID),
 		Prefix: aws.String(reportVersionDir),
@@ -111,15 +112,15 @@ func (c Client) listReportedVersions(ctx context.Context) ([]string, error) {
 }
 
 // findMinVersion finds the minimal version of the given version dates among the latest values in the version window size.
-func (c Client) findMinVersion(ctx context.Context, versionDates []string) (*AzureSEVSNPVersion, string, error) {
+func (c Client) findMinVersion(ctx context.Context, versionDates []string) (AzureSEVSNPVersion, string, error) {
 	var minimalVersion *AzureSEVSNPVersion
 	var minimalDate string
 	sort.Strings(versionDates) // the oldest date with the minimal version should be taken
 	versionDates = versionDates[:c.cacheWindowSize]
 	for _, date := range versionDates {
 		obj, err := client.Fetch(ctx, c.s3Client, reportedAzureSEVSNPVersionAPI{Version: date + ".json"})
 		if err != nil {
-			return nil, "", fmt.Errorf("get object: %w", err)
+			return AzureSEVSNPVersion{}, "", fmt.Errorf("get object: %w", err)
 		}
 
 		if minimalVersion == nil {
@@ -136,7 +137,7 @@ func (c Client) findMinVersion(ctx context.Context, versionDates []string) (*Azu
 			}
 		}
 	}
-	return minimalVersion, minimalDate, nil
+	return *minimalVersion, minimalDate, nil
 }
 
 // isInputNewerThanOtherVersion compares all version fields and returns true if any input field is newer.