jprdonnelly/opentf
1
0
Fork 0
mirror of https://github.com/opentffoundation/opentf.git synced 2025-12-25 01:00:16 -05:00
Code Issues Packages Projects Releases Wiki Activity

getproviders: Support OCI registries that don't report artifactType

Browse Source 
In practical testing I've found that some OCI registry implementations
don't return the artifactType field in their response to a tag resolution
request.

Unfortunately that behavior is ambiguous with what happens when a server
that _does_ support the artifact type describes a manifest that lacks one,
as is true for traditional container images and Helm chart artifacts.

Therefore we must unfortunately sacrifice our specialized error message
that tried to hint about accidentally selecting a container image, and
in that case we'll now return a more generic error about the manifest
having the wrong media type because that's the best we can do with the
information that an OCI registry is _required_ to return from tag
resolution.

Perhaps we'll try to improve on this in a different way later if we find
that folks are often getting confused by this, but for now we'll accept the
slightly-worse error message in this hopefully-rare case so we can
prioritize getting the happy path finished and shipped.

Signed-off-by: Martin Atkins <mart@degeneration.co.uk>
This commit is contained in:
Martin Atkins
2025-03-19 18:05:02 -07:00
parent 5b917e1652
commit 9c5bd4abf9
2 changed files with 27 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
internal/getproviders/oci_registry_mirror_source.go
View File

@@ -387,7 +387,13 @@ func fetchOCIDescriptorForVersion(ctx context.Context, version versions.Version,
if err != nil {
return ociv1.Descriptor{}, fmt.Errorf("resolving tag %q: %w", tagName, err)
}
if desc.ArtifactType != ociIndexManifestArtifactType {
// Not all store implementations can return the manifest's artifact type as part
// of the tag-resolution response, so we'll check this early if we can, but
// if not then we'll check it after we've fetched the manifest instead.
// (Doing this early if possible saves us one request and allows us to return
// a better error message, but we'll still catch a mismatched artifact type
// one way or another.)
if desc.ArtifactType != "" && desc.ArtifactType != ociIndexManifestArtifactType {
switch desc.ArtifactType {
case "application/vnd.opentofu.provider-target":
// We'll get here for an incorrectly-constructed artifact layout where
@@ -399,14 +405,6 @@ func fetchOCIDescriptorForVersion(ctx context.Context, version versions.Version,
// we'll return a specialized error, since confusion between providers
// and modules is common for those new to OpenTofu terminology.
return desc, fmt.Errorf("selected OCI artifact is an OpenTofu module package, not a provider package")
case "":
// Prior to there being an explicit way to represent artifact types earlier
// attempts to adapt OCI Distribution to non-container-image stuff used
// custom layer media types instead. This case also deals with container images
// themselves, which are essentially the "default" kind of artifact. We
// haven't yet fetched the full manifest so we can't actually distinguish
// these from the descriptor alone, and so this error message is generic.
return desc, fmt.Errorf("unsupported OCI artifact type; is this a container image, rather than an OpenTofu provider?")
default:
// For any other artifact type we'll just mention it in the error message
// and hope the reader can figure out what that artifact type represents.
@@ -423,6 +421,10 @@ func fetchOCIDescriptorForVersion(ctx context.Context, version versions.Version,
return desc, fmt.Errorf("unsupported media type %q for OCI index manifest", desc.MediaType)
}
}
// If the server didn't tell us the artifact type it has, then we'll populate
// the artifact type we _want_ and then the manifest fetch will fail if this
// doesn't match.
desc.ArtifactType = ociIndexManifestArtifactType
return desc, nil
}

25
internal/getproviders/oci_registry_mirror_source_test.go
View File

@@ -268,7 +268,14 @@ func TestOCIRegistryMirrorSource(t *testing.T) {
if err == nil {
t.Fatal("unexpected success; want error")
}
if got, want := err.Error(), `unsupported OCI artifact type; is this a container image, rather than an OpenTofu provider?`; got != want {
// Ideally we'd like to return a more helpful error message diagnosing that this
// might be a container image, but we can't really distinguish this case from
// a server that declines to include "artifactType" in a tag resolution response,
// and so we unfortunately end up treating this the same as an incorrectly-constructed
// provider layout with a missing index manifest. Maybe we can find a way to do better
// in future if we find that folks are often confused by this, but we'll be pragmatic
// about it for now.
if got, want := err.Error(), `selected an OCI image manifest directly, but providers must be selected through a multi-platform index manifest`; got != want {
t.Errorf("wrong error\ngot: %s\nwant: %s", got, want)
}
})
@@ -278,14 +285,14 @@ func TestOCIRegistryMirrorSource(t *testing.T) {
if err == nil {
t.Fatal("unexpected success; want error")
}
// Note that the answer to the in the error message is "no" in this case:
// it's not an OpenTofu provider, but it's not a container image either.
// We can't differentiate these cases using only the information available
// in the manifest descriptor, so we assume that accidentally selecting
// a container image is more likely than accidentally selecting a Helm
// chart, but phrase it as a question to hopefully communicate that
// OpenTofu isn't actually sure what the artifact type is.
if got, want := err.Error(), `unsupported OCI artifact type; is this a container image, rather than an OpenTofu provider?`; got != want {
// Ideally we'd like to return a more helpful error message diagnosing that this
// might be a Helm chart artifact, but we can't really distinguish this case from
// a server that declines to include "artifactType" in a tag resolution response,
// and so we unfortunately end up treating this the same as an incorrectly-constructed
// provider layout with a missing index manifest. Maybe we can find a way to do better
// in future if we find that folks are often confused by this, but we'll be pragmatic
// about it for now.
if got, want := err.Error(), `selected an OCI image manifest directly, but providers must be selected through a multi-platform index manifest`; got != want {
t.Errorf("wrong error\ngot: %s\nwant: %s", got, want)
}
})