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.

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

Fix config archs support #162

Merged
merged 4 commits into from
Apr 3, 2022

Conversation

DamianZaremba
Copy link
Contributor

There appears to be 2 issues with the current archs support from the config file.

Currently if you try and specify the archs option in the config file,
the publish command fails along the lines of

  line 7: cannot unmarshal !!int `386` into types.Architecture
  line 7: cannot unmarshal !!str `amd64` into types.Architecture

Add a custom UnmarshalYAML function to Architecture, supporting
decoding the string values into the correct Architecture object.

Currently if we specify 1 arch under bc.ImageConfiguration.Archs, the
publish command will handle it, however if we have more than 1 the
default case uses archs, which is defaulted to all entries (cli
options).

Swap out the default range case to use bc.ImageConfiguration.Archs,
which will be set to archs in the default case (no config specified).

Currently if you try and specify the `archs` option in the config file,
the publish command fails along the lines of
```
  line 7: cannot unmarshal !!int `386` into types.Architecture
  line 7: cannot unmarshal !!str `amd64` into types.Architecture
```

Add a custom `UnmarshalYAML` function to `Architecture`, supporting
decoding the string values into the correct `Architecture` object.
Currently if we specify 1 arch under `bc.ImageConfiguration.Archs`, the
publish command will handle it, however if we have more than 1 the
default case uses `archs`, which is defaulted to all entries (cli
options).

Swap out the default range case to use `bc.ImageConfiguration.Archs`,
which will be set to `archs` in the default case (no config specified).
@DamianZaremba DamianZaremba force-pushed the fix-config-archs-support branch from 72e2625 to ffba058 Compare April 3, 2022 18:40
This will ensure mink exercises all 3 cases of archs handling in the cli
commands.
pkg/build/types/types.go Outdated Show resolved Hide resolved
@kaniini
Copy link
Contributor

kaniini commented Apr 3, 2022

I think that alpine-386_amd64.yaml might be causing problems?

@DamianZaremba
Copy link
Contributor Author

I think that alpine-386_amd64.yaml might be causing problems?

I think everything is good now after chainguard-dev/actions#36, unless I'm missing something.

@kaniini
Copy link
Contributor

kaniini commented Apr 3, 2022

Yeah seems so.

@kaniini kaniini merged commit ee094e7 into chainguard-dev:main Apr 3, 2022
@DamianZaremba DamianZaremba deleted the fix-config-archs-support branch April 3, 2022 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants