Skip to content

Commit

Permalink
Fix for rebase omitting os/arch config properties (#751) (#752)
Browse files Browse the repository at this point in the history
* Quick fix for missing `os` and `architecture` config properties, required by config spec [1]

* Also include unspec'd `os.version` property which is required for Windows images to run on Windows containers [2]

This implementation takes these three properties from newBase, which most accurately defines the newly rebased image's os/architecture. If the properties were taken from original, it would prevent reuse of de-facto ABI-compatible top layers with differing old and new base images (i.e. data-only top layers rebased from amd64 to arm, Windows os.version upgrades).

This does not address additional, optional config properties that are currently being omitted during rebase such as `author` and `created`.

Fixes [#751]

[1]: https://github.com/opencontainers/image-spec/blob/master/config.md#properties
[2]: https://docs.microsoft.com/en-us/virtualization/windowscontainers/deploy-containers/version-compatibility#matching-container-host-version-with-container-image-versions

Signed-off-by: Micah Young <ymicah@vmware.com>
  • Loading branch information
micahyoung committed Aug 5, 2020
1 parent b0d31a1 commit 6d4814a
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 9 deletions.
8 changes: 3 additions & 5 deletions go.mod

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 23 additions & 4 deletions pkg/v1/mutate/rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,42 @@ func Rebase(orig, oldBase, newBase v1.Image) (v1.Image, error) {
return nil, fmt.Errorf("failed to get config for original: %v", err)
}

newConfig, err := newBase.ConfigFile()
if err != nil {
return nil, fmt.Errorf("could not get config for new base: %v", err)
}

// Stitch together an image that contains:
// - original image's config
// - new base image's os/arch properties
// - new base image's layers + top of original image's layers
// - new base image's history + top of original image's history
rebasedImage, err := Config(empty.Image, *origConfig.Config.DeepCopy())
if err != nil {
return nil, fmt.Errorf("failed to create empty image with original config: %v", err)
}

// Add new config properties from existing images.
rebasedConfig, err := rebasedImage.ConfigFile()
if err != nil {
return nil, fmt.Errorf("could not get config for rebased image: %v", err)
}
// OS/Arch properties from new base
rebasedConfig.Architecture = newConfig.Architecture
rebasedConfig.OS = newConfig.OS
rebasedConfig.OSVersion = newConfig.OSVersion

// Apply config properties to rebased.
rebasedImage, err = ConfigFile(rebasedImage, rebasedConfig)
if err != nil {
return nil, fmt.Errorf("failed to replace config for rebased image: %v", err)
}

// Get new base layers and config for history.
newBaseLayers, err := newBase.Layers()
if err != nil {
return nil, fmt.Errorf("could not get new base layers for new base: %v", err)
}
newConfig, err := newBase.ConfigFile()
if err != nil {
return nil, fmt.Errorf("could not get config for new base: %v", err)
}
// Add new base layers.
rebasedImage, err = Append(rebasedImage, createAddendums(0, 0, newConfig.History, newBaseLayers)...)
if err != nil {
Expand Down
25 changes: 25 additions & 0 deletions pkg/v1/mutate/rebase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,20 @@ func TestRebase(t *testing.T) {
t.Log("New base:")
newBaseLayerDigests := layerDigests(t, newBase)

// Add config file os/arch property fields
newBaseConfigFile, err := newBase.ConfigFile()
if err != nil {
t.Fatalf("newBase.ConfigFile: %v", err)
}
newBaseConfigFile.Architecture = "arm"
newBaseConfigFile.OS = "windows"
newBaseConfigFile.OSVersion = "10.0.17763.1339"

newBase, err = mutate.ConfigFile(newBase, newBaseConfigFile)
if err != nil {
t.Fatalf("ConfigFile (newBase): %v", err)
}

// Rebase original image onto new base.
rebased, err := mutate.Rebase(orig, oldBase, newBase)
if err != nil {
Expand Down Expand Up @@ -151,4 +165,15 @@ func TestRebase(t *testing.T) {
t.Errorf("Layer %d mismatch, got %q, want %q", i, got, want)
}
}

// Compare ConfigFile property fields copied from new base.
if rebasedConfig.Architecture != newBaseConfig.Architecture {
t.Errorf("ConfigFile property Architecture mismatch, got %q, want %q", rebasedConfig.Architecture, newBaseConfig.Architecture)
}
if rebasedConfig.OS != newBaseConfig.OS {
t.Errorf("ConfigFile property OS mismatch, got %q, want %q", rebasedConfig.OS, newBaseConfig.OS)
}
if rebasedConfig.OSVersion != newBaseConfig.OSVersion {
t.Errorf("ConfigFile property OSVersion mismatch, got %q, want %q", rebasedConfig.OSVersion, newBaseConfig.OSVersion)
}
}

0 comments on commit 6d4814a

Please sign in to comment.