Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

go.mod: update github.com/containers/image/v5 #2925

Merged
merged 2 commits into from
Aug 29, 2022
Merged

go.mod: update github.com/containers/image/v5 #2925

merged 2 commits into from
Aug 29, 2022

Conversation

ondrejbudai
Copy link
Member

@ondrejbudai ondrejbudai commented Aug 28, 2022 •
edited
Loading

TL;DR: This fixes broken unit tests and other code using container.NewClient when running on EL9 with containers-common>=1-43.el9 and on EL8 with containers-common>=1-40.el8. We will definitely be backporting this to 9.1 and 8.7.

Version 5.22 introduced a new option to /etc/containers/policy.json called keyPaths, see containers/image#1609

EL9 immediately took advantage of this new feature and started using it, see https://gitlab.com/redhat/centos-stream/rpms/containers-common/-/commit/04645c4a84442da3324eea8f6538a5768e69919a

This quickly became an issue in our code: The go library (containers/image) parses the configuration file very strictly and refuses to create a client when policy.json with an unknown key is present on the filesystem. As we used 5.21.1 that doesn't know the new key, our unit tests started to failing when newer containers-common is present.

Reproducer:

podman run --pull=always --rm -it centos:stream9
dnf install -y dnf-plugins-core
dnf config-manager --set-enabled crb
dnf install -y gpgme-devel libassuan-devel krb5-devel golang git-core
git clone https://github.com/osbuild/osbuild-composer
cd osbuild-composer

# install the new containers-common and run the test
dnf install -y https://kojihub.stream.centos.org/kojifiles/packages/containers-common/1/44.el9/x86_64/containers-common-1-44.el9.x86_64.rpm
go test -count 1 ./...

# this returns:
--- FAIL: TestClientResolve (0.00s)
    client_test.go:31:
        	Error Trace:	client_test.go:31
        	Error:      	Received unexpected error:
        	            	Unknown key "keyPaths"
        	            	invalid policy in "/etc/containers/policy.json"
        	            	github.com/containers/image/v5/signature.NewPolicyFromFile
        	            		/osbuild-composer/vendor/github.com/containers/image/v5/signature/policy_config.go:88
        	            	github.com/osbuild/osbuild-composer/internal/container.NewClient
        	            		/osbuild-composer/internal/container/client.go:123
        	            	github.com/osbuild/osbuild-composer/internal/container_test.TestClientResolve
        	            		/osbuild-composer/internal/container/client_test.go:29
        	            	testing.tRunner
        	            		/usr/lib/golang/src/testing/testing.go:1439
        	            	runtime.goexit
        	            		/usr/lib/golang/src/runtime/asm_amd64.s:1571
        	Test:       	TestClientResolve
    client_test.go:32:
        	Error Trace:	client_test.go:32
        	Error:      	Expected value not to be nil.
        	Test:       	TestClientResolve

 When run with an older containers-common, it succeeds:
 dnf install -y https://kojihub.stream.centos.org/kojifiles/packages/containers-common/1/40.el9/x86_64/containers-common-1-40.el9.x86_64.rpm
 go test -count 1 ./...
 PASS

To sum it up, I had to upgrade github.com/containers/image/v5 to v5.22.0. Unfortunately, this wasn't so simple, see

go get github.com/containers/image/v5@latest
go: github.com/containers/image/v5@v5.22.0 requires
	github.com/letsencrypt/boulder@v0.0.0-20220331220046-b23ab962616e requires
	github.com/honeycombio/beeline-go@v1.1.1 requires
	github.com/gobuffalo/pop/v5@v5.3.1 requires
	github.com/mattn/go-sqlite3@v2.0.3+incompatible: reading github.com/mattn/go-sqlite3/go.mod at revision v2.0.3: unknown revision v2.0.3

It turns out that github.com/mattn/go-sqlite3@v2.0.3+incompatible has been recently retracted, see mattn/go-sqlite3#998. This broke a ton of packages depending on it. I was able to fix it by adding

exclude github.com/mattn/go-sqlite3 v2.0.3+incompatible

to our go.mod, see mattn/go-sqlite3#975 (comment)

After adding it, go get github.com/containers/image/v5@latest succeeded and tools/prepare-source.sh took care of the rest.

Signed-off-by: Ondřej Budai ondrej@budai.cz

This pull request includes:

  • adequate testing for the new functionality or fixed issue
  • adequate documentation informing people about the change such as

@ondrejbudai
blueprint: remove omitempty from Customizations
b6297c9 
See BurntSushi/toml#360

A recent change in BurntSushi/toml made encoding fail (later changed to
error) if a struct is marked as omitempty and is comparable. Go docs about
equality: https://go.dev/doc/go1#equality. Basically: A struct is comparable
if all of its fields are comparable. Slices are not comparable.

Customizations are marked as omitempty but they contain a lot of slices,
thus they are not comparable. The new version of BurntSushi/toml therefore
panics when we encode them.

The solution is to remove the omitempty tag from Customizations.

Signed-off-by: Ondřej Budai <ondrej@budai.cz>
@ondrejbudai
go.mod: update github.com/containers/image/v5
7e274d8 
Version 5.22 introduced a new option to /etc/containers/policy.json called
keyPaths, see

containers/image#1609

EL9 immediately took advantage of this new feature and started using it, see
https://gitlab.com/redhat/centos-stream/rpms/containers-common/-/commit/04645c4a84442da3324eea8f6538a5768e69919a

This quickly became an issue in our code: The go library (containers/image)
parses the configuration file very strictly and refuses to create a client
when policy.json with an unknown key is present on the filesystem. As we
used 5.21.1 that doesn't know the new key, our unit tests started to
failing when containers-common was present.

Reproducer:
podman run --pull=always --rm -it centos:stream9
dnf install -y dnf-plugins-core
dnf config-manager --set-enabled crb
dnf install -y gpgme-devel libassuan-devel krb5-devel golang git-core
git clone https://github.com/osbuild/osbuild-composer
cd osbuild-composer

# install the new containers-common and run the test
dnf install -y https://kojihub.stream.centos.org/kojifiles/packages/containers-common/1/44.el9/x86_64/containers-common-1-44.el9.x86_64.rpm
go test -count 1 ./...

# this returns:
--- FAIL: TestClientResolve (0.00s)
    client_test.go:31:
        	Error Trace:	client_test.go:31
        	Error:      	Received unexpected error:
        	            	Unknown key "keyPaths"
        	            	invalid policy in "/etc/containers/policy.json"
        	            	github.com/containers/image/v5/signature.NewPolicyFromFile
        	            		/osbuild-composer/vendor/github.com/containers/image/v5/signature/policy_config.go:88
        	            	github.com/osbuild/osbuild-composer/internal/container.NewClient
        	            		/osbuild-composer/internal/container/client.go:123
        	            	github.com/osbuild/osbuild-composer/internal/container_test.TestClientResolve
        	            		/osbuild-composer/internal/container/client_test.go:29
        	            	testing.tRunner
        	            		/usr/lib/golang/src/testing/testing.go:1439
        	            	runtime.goexit
        	            		/usr/lib/golang/src/runtime/asm_amd64.s:1571
        	Test:       	TestClientResolve
    client_test.go:32:
        	Error Trace:	client_test.go:32
        	Error:      	Expected value not to be nil.
        	Test:       	TestClientResolve

 When run with an older containers-common, it succeeds:
 dnf install -y https://kojihub.stream.centos.org/kojifiles/packages/containers-common/1/40.el9/x86_64/containers-common-1-40.el9.x86_64.rpm
 go test -count 1 ./...
 PASS

To sum it up, I had to upgrade github.com/containers/image/v5 to v5.22.0.
Unfortunately, this wasn't so simple, see

go get github.com/containers/image/v5@latest
go: github.com/containers/image/v5@v5.22.0 requires
	github.com/letsencrypt/boulder@v0.0.0-20220331220046-b23ab962616e requires
	github.com/honeycombio/beeline-go@v1.1.1 requires
	github.com/gobuffalo/pop/v5@v5.3.1 requires
	github.com/mattn/go-sqlite3@v2.0.3+incompatible: reading github.com/mattn/go-sqlite3/go.mod at revision v2.0.3: unknown revision v2.0.3

It turns out that github.com/mattn/go-sqlite3@v2.0.3+incompatible has been
recently retracted mattn/go-sqlite3#998 and this
broke a ton of packages depending on it. I was able to fix it by adding

exclude github.com/mattn/go-sqlite3 v2.0.3+incompatible

to our go.mod, see
mattn/go-sqlite3#975 (comment)

After adding it,
go get github.com/containers/image/v5@latest
succeeded and tools/prepare-source.sh took care of the rest.

Signed-off-by: Ondřej Budai <ondrej@budai.cz>
@ondrejbudai
Copy link
Member Author

ondrejbudai commented Aug 28, 2022 •
edited
Loading

I had to add yet another commit to fix a new "feature" of the updated BurntSushi/toml that got pulled in by updating github.com/containers/image/v5. 🤷 See the first commit for more details.

Anyway, I tested this PR on CentOS Stream 9 with containers-common-1-44.el9.x86_64 installed and the unit tests are now working.

[root@e9bdfe85ad86 osbuild-composer]# rpm -q containers-common
containers-common-1-44.el9.x86_64
[root@e9bdfe85ad86 osbuild-composer]# git fetch origin pull/2925/head:update
remote: Enumerating objects: 632, done.
remote: Counting objects: 100% (279/279), done.
remote: Compressing objects: 100% (8/8), done.
remote: Total 632 (delta 271), reused 273 (delta 271), pack-reused 353
Receiving objects: 100% (632/632), 1011.65 KiB | 1.90 MiB/s, done.
Resolving deltas: 100% (337/337), completed with 217 local objects.
From https://github.com/osbuild/osbuild-composer
 * [new ref]           refs/pull/2925/head -> update
[root@e9bdfe85ad86 osbuild-composer]# git checkout update
Switched to branch 'update'
[root@e9bdfe85ad86 osbuild-composer]# go test -count 1 ./...
?   	github.com/osbuild/osbuild-composer/cmd/gen-manifests	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/mock-dnf-json	[no test files]
ok  	github.com/osbuild/osbuild-composer/cmd/osbuild-composer	0.055s
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-composer-image-definitions	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-koji	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-mock-openid-provider	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-package-sets	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-pipeline	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-playground	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-service-maintenance	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-store-dump	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-upload-aws	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-upload-azure	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-upload-container	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-upload-gcp	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-upload-generic-s3	[no test files]
?   	github.com/osbuild/osbuild-composer/cmd/osbuild-upload-oci	[no test files]
ok  	github.com/osbuild/osbuild-composer/cmd/osbuild-worker	0.099s
?   	github.com/osbuild/osbuild-composer/internal/artifact	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/auth	0.021s
ok  	github.com/osbuild/osbuild-composer/internal/blueprint	0.009s
ok  	github.com/osbuild/osbuild-composer/internal/client	0.820s
?   	github.com/osbuild/osbuild-composer/internal/cloud/awscloud	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/cloud/gcp	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/cloudapi	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/cloudapi/v2	2.420s
ok  	github.com/osbuild/osbuild-composer/internal/common	0.011s
ok  	github.com/osbuild/osbuild-composer/internal/container	0.335s
ok  	github.com/osbuild/osbuild-composer/internal/crypt	0.016s
ok  	github.com/osbuild/osbuild-composer/internal/disk	0.029s
ok  	github.com/osbuild/osbuild-composer/internal/distro	12.327s
?   	github.com/osbuild/osbuild-composer/internal/distro/distro_test_common	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/distro/fedora	0.036s
ok  	github.com/osbuild/osbuild-composer/internal/distro/rhel7	0.021s
ok  	github.com/osbuild/osbuild-composer/internal/distro/rhel8	0.063s
ok  	github.com/osbuild/osbuild-composer/internal/distro/rhel9	0.056s
?   	github.com/osbuild/osbuild-composer/internal/distro/test_distro	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/distroregistry	0.015s
ok  	github.com/osbuild/osbuild-composer/internal/dnfjson	1.683s
?   	github.com/osbuild/osbuild-composer/internal/environment	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/image	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/jobqueue/fsjobqueue	0.467s
?   	github.com/osbuild/osbuild-composer/internal/jobqueue/jobqueuetest	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/jsondb	0.005s
?   	github.com/osbuild/osbuild-composer/internal/manifest	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/mocks/distro	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/mocks/dnfjson	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/mocks/rpmmd	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/mocks/rpmrepo	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/osbuild	0.145s
?   	github.com/osbuild/osbuild-composer/internal/oscap	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/ostree	0.004s
?   	github.com/osbuild/osbuild-composer/internal/ostree/mock_ostree_repo	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/platform	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/prometheus	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/reporegistry	0.030s
ok  	github.com/osbuild/osbuild-composer/internal/rhsm	0.004s
ok  	github.com/osbuild/osbuild-composer/internal/rpmmd	0.016s
ok  	github.com/osbuild/osbuild-composer/internal/rpmmd/test	0.012s
?   	github.com/osbuild/osbuild-composer/internal/runner	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/store	0.020s
ok  	github.com/osbuild/osbuild-composer/internal/target	0.012s
?   	github.com/osbuild/osbuild-composer/internal/test	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/upload/azure	0.010s
?   	github.com/osbuild/osbuild-composer/internal/upload/koji	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/upload/oci	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/upload/vmware	[no test files]
ok  	github.com/osbuild/osbuild-composer/internal/weldr	0.530s
ok  	github.com/osbuild/osbuild-composer/internal/worker	0.067s
?   	github.com/osbuild/osbuild-composer/internal/worker/api	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/worker/clienterrors	[no test files]
?   	github.com/osbuild/osbuild-composer/internal/workload	[no test files]
?   	github.com/osbuild/osbuild-composer/pkg/jobqueue	[no test files]
?   	github.com/osbuild/osbuild-composer/pkg/jobqueue/dbjobqueue	[no test files]

@thozza thozza merged commit 29f66a2 into osbuild:main Aug 29, 2022
@ondrejbudai ondrejbudai deleted the update branch August 29, 2022 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thozza thozza thozza approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ondrejbudai @thozza