imagebuildah: do not depend on go/types

[Luap99]

This package is quite large, this import costs us 1.4MB in binary size. 1MB when stip'ed.

I am not sure what the point of validation the architecture like this is. This seems to limit the build to architectures known by go which may not be the same as architectures users try to build. The commit 98f8707 which added this check doesn't seem to add an actual reason either why we check the go compiler architecture list either.

Instead use the some logic like for the OS and only reject an empty or "unknown" value.

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [Luap99]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[nalind]

#5335 references #5331.

[Luap99]

#5335 references #5331.

Yeah but #5331 list only "unknown" as arch which this patch still excludes so there should be no real chnage for that use case.

I don't see what we gain by filtering by known arches. Bloating the binary by a megabyte for a list of 16 arches seems a bit over the top, if we want to keep it I rather "copy" such list.

[nalind]

"unknown" is used for the platform OS and arch value for entries in image indexes in Docker Hub which have attestations in them, even though the platform field is itself optional. I guess this'll work unless/until some other value starts showing up there.

[Luap99]

rebased

"unknown" is used for the platform OS and arch value for entries in image indexes in Docker Hub which have attestations in them, even though the platform field is itself optional. I guess this'll work unless/until some other value starts showing up there.

Well then people are missusing the standards, no? special coding "unkown" should be enough. If people expect --all-platforms to work on an image that contain bogus platforms then that seems just normal to error?

Also IMO the code is already wrong as is. Only allowing the arches go supports is wrong, if I just do ls /proc/sys/fs/binfmt_misc/ there are a lot more emulators for other arches available than what go supports AFAICS so to me this here is the right fix regadless.

[mtrmac]

Also IMO the code is already wrong as is. Only allowing the arches go supports is wrong, if I just do ls /proc/sys/fs/binfmt_misc/ there are a lot more emulators for other arches available than what go supports AFAICS so to me this here is the right fix regadless.

The https://github.com/opencontainers/image-spec/blob/main/image-index.md spec defines the architectures to be what GOARCH uses; so, in that sense, referring to architectures supported by Go is correct.

I would quibble with the go/types approach in that go/types lists the platforms currently implemented, but conceptually we want all GOARCH values that were ever defined, whether past and deprecated, or future where Go is only to be bootstrapped later. I.e, ideally , we want Go’s https://github.com/golang/go/blob/66536242fce34787230c42078a7bbd373ef8dcb0/src/internal/syslist/syslist.go#L58 . But I can’t see any public API referring to this, so that’s moot.

---

I suppose we could hard-code a list (and add a CI test that validates it against the Go sources????). I don’t like that approach enough to advocate for it, but also wouldn’t object too much.

… we are already hard-coding c/common/pkg/completion/AutocompleteArch. Obviously we don’t want to refer to exactly that from here, but, maybe, hard-coding is not so unthinkable.

[Luap99]

The https://github.com/opencontainers/image-spec/blob/main/image-index.md spec defines the architectures to be what GOARCH uses; so, in that sense, referring to architectures supported by Go is correct.

Huh, so you are telling me nobody can use OCI containers for architectures not support by the go compiler at least when taking that standard literately? That seems bizarre to me?

The way I read this the spec says SHOULD which per https://datatracker.ietf.org/doc/html/rfc2119#section-3 (linked form the image spec as definition of the words) means:

3. SHOULD This word, or the adjective "RECOMMENDED", mean that there
may exist valid reasons in particular circumstances to ignore a
particular item, but the full implications must be understood and
carefully weighed before choosing a different course.

I think there is a decent case to not require all images to only use go approved arch names, that feels like an arbitrary restriction to me we should not impose on users.

For what is worth we are violating that with our own podman-machine-os manifest list which uses aarch64 and x86_64 for the disk artifacts.

I would quibble with the go/types approach in that go/types lists the platforms currently implemented, but conceptually we want all GOARCH values that were ever defined, whether past and deprecated, or future where Go is only to be bootstrapped later. I.e, ideally , we want Go’s https://github.com/golang/go/blob/66536242fce34787230c42078a7bbd373ef8dcb0/src/internal/syslist/syslist.go#L58 . But I can’t see any public API referring to this, so that’s moot.

yeah but it doesn't address the case of using arches not supported by go and I would think there must be some small group of people wanting to use OCI stuff on such arches?. And even then any list we choose will be valid today but we cannot predict what arches are added in the future and IMO it seems odd to require a new buildah update to allow building a new arch?

---

… we are already hard-coding c/common/pkg/completion/AutocompleteArch. Obviously we don’t want to refer to exactly that from here, but, maybe, hard-coding is not so unthinkable.

Well sure except this is a nice UI thing and not something actually enforced anywhere so if it isn't 100% right there is basically no harm done.

[Luap99]

Anyway if the consensus is we should have a hard coded allow list I am fine to implement that as well even though I personally think it is the wrong approach.

[mtrmac]

I think where I’m at is that go/types is not exactly the right thing so I’m not strongly advocating for the code to remain as is.

As for whether to hard-code a list of recognized platforms, or whether to just exclude some specific invalid ones, I don’t feel strongly. Making it hard for users to accidentally use x86_64 seems valuable, OTOH committing to maintain a hard-coded list is extra work.

[github-actions]

A friendly reminder that this PR had no activity for 30 days.