Make artifacts ingester work with both GitHub and OCI providers (#3309)

* Make artifacts ingester work with both GitHub and OCI providers

This modifies the ingester to change its behavior depending on whether
the provider is an OCI provider or a github one.

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>

* Pass in OCI authenticator to container verifier

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>

* Fix getting registry from OCI provider

It's used differently than I thought

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>

* Use tags as versions

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>

* Fix mindev

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>

* Use a vesrsion name dict for github compatibility

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>

* Run `go mod tidy`

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>

* Fix dockerhub rule evaluation and rename versions

The logic to refer to containers is now changed to using the `Sha` field
from the ArtifactVersions since this is what the bebavior was. With
this, we can now evaluate policies with the dockerhub provider!!

The usage of the word version around the verifier was misleading, so
this was fixed so the code and expectations are easier to follow.

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>

---------

Signed-off-by: Juan Antonio Osorio <ozz@stacklok.com>

cmd/dev/app/image/list_tags.go

@@ -37,6 +37,7 @@ func CmdListTags() *cobra.Command {
 	}
 
 	listCmd.Flags().StringP("base-url", "b", "", "base URL for the OCI registry")
+	listCmd.Flags().StringP("owner", "o", "", "owner of the artifact")
 	listCmd.Flags().StringP("container", "c", "", "container name to list tags for")
 	//nolint:goconst // let's not use a const for this one
 	listCmd.Flags().StringP("token", "t", "", "token to authenticate to the provider."+
@@ -57,6 +58,7 @@ func runCmdListTags(cmd *cobra.Command, _ []string) error {
 
 	// get the provider
 	baseURL := cmd.Flag("base-url")
+	owner := cmd.Flag("owner")
 	contname := cmd.Flag("container")
 
 	if baseURL.Value.String() == "" {
@@ -66,8 +68,10 @@ func runCmdListTags(cmd *cobra.Command, _ []string) error {
 		return fmt.Errorf("container name is required")
 	}
 
+	regWithOwner := fmt.Sprintf("%s/%s", owner.Value.String(), baseURL.Value.String())
+
 	cred := credentials.NewOAuth2TokenCredential(viper.GetString("auth.token"))
-	prov := oci.New(cred, baseURL.Value.String())
+	prov := oci.New(cred, baseURL.Value.String(), regWithOwner)
 
 	// get the containers
 	containers, err := prov.ListTags(ctx, contname.Value.String())

go.mod

@@ -254,7 +254,7 @@ require (
 	github.com/muesli/termenv v0.15.2 // indirect
 	github.com/oklog/ulid v1.3.1 // indirect
 	github.com/opencontainers/go-digest v1.0.0
-	github.com/opencontainers/image-spec v1.1.0 // indirect
+	github.com/opencontainers/image-spec v1.1.0
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
 	github.com/pelletier/go-toml/v2 v2.1.1 // indirect
 	github.com/pjbgf/sha1cd v0.3.0 // indirect

internal/engine/ingester/artifact/artifact.go

@@ -19,8 +19,6 @@ package artifact
 import (
 	"context"
 	"fmt"
-	"net/url"
-	"sort"
 	"strings"
 	"time"
 
@@ -50,7 +48,7 @@ const (
 // Ingest is the engine for a rule type that uses artifact data ingest
 // Implements enginer.ingester.Ingester
 type Ingest struct {
-	ghCli provifv1.GitHub
+	prov provifv1.Provider
 
 	// artifactVerifier is the verifier for sigstore. It's only used in the Ingest method
 	// but we store it in the Ingest structure to allow tests to set a custom artifactVerifier
@@ -74,9 +72,9 @@ type verifiedAttestation struct {
 }
 
 // NewArtifactDataIngest creates a new artifact rule data ingest engine
-func NewArtifactDataIngest(ghCli provifv1.GitHub) (*Ingest, error) {
+func NewArtifactDataIngest(prov provifv1.Provider) (*Ingest, error) {
 	return &Ingest{
-		ghCli: ghCli,
+		prov: prov,
 	}, nil
 }
 
@@ -124,24 +122,23 @@ func (i *Ingest) getApplicableArtifactVersions(
 	artifact *pb.Artifact,
 	cfg *ingesterConfig,
 ) ([]map[string]any, error) {
-	// Make sure the artifact type matches
-	if newArtifactIngestType(artifact.Type) != cfg.Type {
-		return nil, evalerrors.NewErrEvaluationSkipSilently("artifact type mismatch")
+	if err := validateConfiguration(artifact, cfg); err != nil {
+		return nil, err
 	}
 
-	// If a name is specified, make sure it matches
-	if cfg.Name != "" && cfg.Name != artifact.Name {
-		return nil, evalerrors.NewErrEvaluationSkipSilently("artifact name mismatch")
+	vers, err := getVersioner(i.prov, artifact)
+	if err != nil {
+		return nil, err
 	}
 
-	// Get all artifact versions filtering out those that don't apply to this rule
-	versions, err := getAndFilterArtifactVersions(ctx, cfg, i.ghCli, artifact)
+	// Get all artifact checksums filtering out those that don't apply to this rule
+	checksums, err := getAndFilterArtifactVersions(ctx, cfg, vers, artifact)
 	if err != nil {
 		return nil, err
 	}
 
 	// Get the provenance info for all artifact versions that apply to this rule
-	verificationResults, err := i.getVerificationResult(ctx, cfg, artifact, versions)
+	verificationResults, err := i.getVerificationResult(ctx, cfg, artifact, checksums)
 	if err != nil {
 		return nil, err
 	}
@@ -160,11 +157,32 @@ func (i *Ingest) getApplicableArtifactVersions(
 	return result, nil
 }
 
+func validateConfiguration(
+	artifact *pb.Artifact,
+	cfg *ingesterConfig,
+) error {
+	// Make sure the artifact type matches
+	if newArtifactIngestType(artifact.Type) != cfg.Type {
+		return evalerrors.NewErrEvaluationSkipSilently("artifact type mismatch")
+	}
+
+	if cfg.Type != artifactTypeContainer {
+		return evalerrors.NewErrEvaluationSkipSilently("only container artifacts are supported at the moment")
+	}
+
+	// If a name is specified, make sure it matches
+	if cfg.Name != "" && cfg.Name != artifact.Name {
+		return evalerrors.NewErrEvaluationSkipSilently("artifact name mismatch")
+	}
+
+	return nil
+}
+
 func (i *Ingest) getVerificationResult(
 	ctx context.Context,
 	cfg *ingesterConfig,
 	artifact *pb.Artifact,
-	versions []string,
+	checksums []string,
 ) ([]verification, error) {
 	var versionResults []verification
 	// Get the verifier for sigstore
@@ -174,21 +192,21 @@ func (i *Ingest) getVerificationResult(
 	}
 
 	// Loop through all artifact versions that apply to this rule and get the provenance info for each
-	for _, artifactVersion := range versions {
+	for _, artifactChecksum := range checksums {
 		// Try getting provenance info for the artifact version
 		results, err := artifactVerifier.Verify(ctx, verifyif.ArtifactTypeContainer,
-			artifact.Owner, artifact.Name, artifactVersion)
+			artifact.Owner, artifact.Name, artifactChecksum)
 		if err != nil {
 			// We consider err != nil as a fatal error, so we'll fail the rule evaluation here
-			artifactName := container.BuildImageRef("", artifact.Owner, artifact.Name, artifactVersion)
+			artifactName := container.BuildImageRef("", artifact.Owner, artifact.Name, artifactChecksum)
 			zerolog.Ctx(ctx).Debug().Err(err).Str("name", artifactName).Msg("failed getting signature information")
 			return nil, fmt.Errorf("failed getting signature information: %w", err)
 		}
 		// Loop through all results and build the verification result for each
 		for _, res := range results {
 			// Log a debug message in case we failed to find or verify any signature information for the artifact version
 			if !res.IsSigned || !res.IsVerified {
-				artifactName := container.BuildImageRef("", artifact.Owner, artifact.Name, artifactVersion)
+				artifactName := container.BuildImageRef("", artifact.Owner, artifact.Name, artifactChecksum)
 				zerolog.Ctx(ctx).Debug().Str("name", artifactName).Msg("failed to find or verify signature information")
 			}
 
@@ -230,21 +248,45 @@ func getVerifier(i *Ingest, cfg *ingesterConfig) (verifyif.ArtifactVerifier, err
 		return i.artifactVerifier, nil
 	}
 
+	verifieropts := []container.AuthMethod{}
+	if i.prov.CanImplement(pb.ProviderType_PROVIDER_TYPE_GITHUB) {
+		ghcli, err := provifv1.As[provifv1.GitHub](i.prov)
+		if err != nil {
+			return nil, fmt.Errorf("unable to get github provider from provider configuration")
+		}
+		verifieropts = append(verifieropts, container.WithGitHubClient(ghcli))
+	} else if i.prov.CanImplement(pb.ProviderType_PROVIDER_TYPE_OCI) {
+		ocicli, err := provifv1.As[provifv1.OCI](i.prov)
+		if err != nil {
+			return nil, fmt.Errorf("unable to get oci provider from provider configuration")
+		}
+		cauthn, err := ocicli.GetAuthenticator()
+		if err != nil {
+			return nil, fmt.Errorf("unable to get oci authenticator: %w", err)
+		}
+		verifieropts = append(verifieropts, container.WithRegistry(ocicli.GetRegistry()),
+			container.WithAuthenticator(cauthn))
+	}
+
 	artifactVerifier, err := verifier.NewVerifier(
 		verifier.VerifierSigstore,
 		cfg.Sigstore,
-		container.WithGitHubClient(i.ghCli))
+		verifieropts...,
+	)
 	if err != nil {
 		return nil, fmt.Errorf("error getting sigstore verifier: %w", err)
 	}
 
 	return artifactVerifier, nil
 }
 
+// getAndFilterArtifactVersions fetches the available versions and filters the
+// ones that apply to the rule. Note that this returns the checksums of the
+// applicable artifact versions.
 func getAndFilterArtifactVersions(
 	ctx context.Context,
 	cfg *ingesterConfig,
-	ghCli provifv1.GitHub,
+	vers versioner,
 	artifact *pb.Artifact,
 ) ([]string, error) {
 	var res []string
@@ -256,32 +298,30 @@ func getAndFilterArtifactVersions(
 	}
 
 	// Fetch all available versions of the artifact
-	artifactName := url.QueryEscape(artifact.GetName())
-	upstreamVersions, err := ghCli.GetPackageVersions(
-		ctx, artifact.Owner, artifact.GetTypeLower(), artifactName,
-	)
+	upstreamVersions, err := vers.GetVersions(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("error retrieving artifact versions: %w", err)
 	}
 
+	name := artifact.GetName()
+
 	// Loop through all and filter out the versions that don't apply to this rule
 	for _, version := range upstreamVersions {
-		tags := version.Metadata.Container.Tags
-		sort.Strings(tags)
-
 		// Decide if the artifact version should be skipped or not
-		err = isSkippable(verifyif.ArtifactTypeContainer, version.CreatedAt.Time, map[string]interface{}{"tags": tags}, filter)
+		tags := version.GetTags()
+		tagsopt := map[string]interface{}{"tags": tags}
+		err = isSkippable(version.GetCreatedAt().AsTime(), tagsopt, filter)
 		if err != nil {
-			zerolog.Ctx(ctx).Debug().Str("name", *version.Name).Strs("tags", tags).Str(
+			zerolog.Ctx(ctx).Debug().Str("name", name).Strs("tags", tags).Str(
 				"reason",
 				err.Error(),
 			).Msg("skipping artifact version")
 			continue
 		}
 
 		// If the artifact version is applicable to this rule, add it to the list
-		zerolog.Ctx(ctx).Debug().Str("name", *version.Name).Strs("tags", tags).Msg("artifact version matched")
-		res = append(res, *version.Name)
+		zerolog.Ctx(ctx).Debug().Str("name", name).Strs("tags", tags).Msg("artifact version matched")
+		res = append(res, version.Sha)
 	}
 
 	// If no applicable artifact versions were found for this rule, we can go ahead and fail the rule evaluation here
@@ -299,33 +339,29 @@ var (
 )
 
 // isSkippable determines if an artifact should be skipped
+// Note this is only applicable to container artifacts.
 // TODO - this should be refactored as well, for now just a forklift from reconciler
-func isSkippable(artifactType verifyif.ArtifactType, createdAt time.Time, opts map[string]interface{}, filter tagMatcher) error {
-	switch artifactType {
-	case verifyif.ArtifactTypeContainer:
-		// if the artifact is older than the retention period, skip it
-		if createdAt.Before(ArtifactTypeContainerRetentionPeriod) {
-			return fmt.Errorf("artifact is older than retention period - %s", ArtifactTypeContainerRetentionPeriod)
-		}
-		tags, ok := opts["tags"].([]string)
-		if !ok {
-			return nil
-		} else if len(tags) == 0 {
-			// if the artifact has no tags, skip it
-			return fmt.Errorf("artifact has no tags")
-		}
-		// if the artifact has a .sig tag it's a signature, skip it
-		if verifier.GetSignatureTag(tags) != "" {
-			return fmt.Errorf("artifact is a signature")
-		}
-		// if the artifact tags don't match the tag matcher, skip it
-		if !filter.MatchTag(tags...) {
-			return fmt.Errorf("artifact tags does not match")
-		}
-		return nil
-	default:
+func isSkippable(createdAt time.Time, opts map[string]interface{}, filter tagMatcher) error {
+	// if the artifact is older than the retention period, skip it
+	if createdAt.Before(ArtifactTypeContainerRetentionPeriod) {
+		return fmt.Errorf("artifact is older than retention period - %s", ArtifactTypeContainerRetentionPeriod)
+	}
+	tags, ok := opts["tags"].([]string)
+	if !ok {
 		return nil
+	} else if len(tags) == 0 {
+		// if the artifact has no tags, skip it
+		return fmt.Errorf("artifact has no tags")
+	}
+	// if the artifact has a .sig tag it's a signature, skip it
+	if verifier.GetSignatureTag(tags) != "" {
+		return fmt.Errorf("artifact is a signature")
 	}
+	// if the artifact tags don't match the tag matcher, skip it
+	if !filter.MatchTag(tags...) {
+		return fmt.Errorf("artifact tags does not match")
+	}
+	return nil
 }
 
 func branchFromRef(ref string) string {

internal/engine/ingester/artifact/artifact_test.go

@@ -569,7 +569,7 @@ func TestArtifactIngestMatching(t *testing.T) {
 			ing, err := NewArtifactDataIngest(prov)
 			require.NoError(t, err, "expected no error")
 
-			ing.ghCli = mockGhClient
+			ing.prov = mockGhClient
 			ing.artifactVerifier = mockVerifier
 
 			tt.mockSetup(mockGhClient, mockVerifier)