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

Adds setters for OS/OSVersion/Architecture #66

Merged
merged 2 commits into from
Oct 15, 2020
Merged

Adds setters for OS/OSVersion/Architecture #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec9e535
Adds setters for OS/OSVersion/Architecture to image
Oct 8, 2020
bd9808c
Change local SetOS to accept only valid values
Oct 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions fakes/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func (i *Image) OS() (string, error) {
}

func (i *Image) OSVersion() (string, error) {
return i.os, nil
return i.osVersion, nil
}

func (i *Image) Architecture() (string, error) {
Expand Down Expand Up @@ -122,6 +122,21 @@ func (i *Image) SetEnv(k string, v string) error {
return nil
}

func (i *Image) SetOS(o string) error {
i.os = o
return nil
}

func (i *Image) SetOSVersion(v string) error {
i.osVersion = v
return nil
}

func (i *Image) SetArchitecture(a string) error {
i.architecture = a
return nil
}

func (i *Image) SetWorkingDir(dir string) error {
i.workingDir = dir
return nil
Expand Down Expand Up @@ -268,12 +283,6 @@ func (i *Image) SetIdentifier(identifier imgutil.Identifier) {
i.identifier = identifier
}

func (i *Image) SetPlatform(os, osVersion, architecture string) {
i.os = os
i.osVersion = osVersion
i.architecture = architecture
}

func (i *Image) Cleanup() error {
return os.RemoveAll(i.layerDir)
}
Expand Down
3 changes: 3 additions & 0 deletions image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ type Image interface {
SetEntrypoint(...string) error
SetWorkingDir(string) error
SetCmd(...string) error
SetOS(string) error
SetOSVersion(string) error
SetArchitecture(string) error
Comment on lines +41 to +43
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually I feel like these options make more sense as options on NewImage. Having constructor options instead of a setter would convey that is not valid to change these values for any image other than an empty image. But I can see how the awkwardness of our existing FromBase option makes that difficult to support without broader changes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good call. I was also wondering how it will intersect with the needs of #54 which would also need very similar looking flags but to instead filter the FromBaseImage platform.

Rebase(string, Image) error
AddLayer(path string) error
AddLayerWithDiffID(path, diffID string) error
Expand Down
17 changes: 17 additions & 0 deletions local/local.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,23 @@ func (i *Image) SetLabel(key, val string) error {
return nil
}

func (i *Image) SetOS(osVal string) error {
if osVal != i.inspect.Os {
return fmt.Errorf(`invalid os: must match the daemon: "%s"`, i.inspect.Os)
}
return nil
ekcasey marked this conversation as resolved.
Show resolved Hide resolved
}

func (i *Image) SetOSVersion(osVersion string) error {
i.inspect.OsVersion = osVersion
return nil
}

func (i *Image) SetArchitecture(architecture string) error {
i.inspect.Architecture = architecture
return nil
}

func (i *Image) RemoveLabel(key string) error {
delete(i.inspect.Config.Labels, key)
return nil
Expand Down
53 changes: 53 additions & 0 deletions local/local_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,59 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
})
})

when("#SetOS", func() {
var repoName = newTestImageName()

it.After(func() {
h.AssertNil(t, h.DockerRmi(dockerClient, repoName))
})

it("allows noop sets for values that match the daemon", func() {
img, err := local.NewImage(repoName, dockerClient)
h.AssertNil(t, err)

err = img.SetOS("fakeos")
h.AssertError(t, err, "invalid os: must match the daemon")

err = img.SetOS(daemonOS)
h.AssertNil(t, err)

h.AssertNil(t, img.Save())

inspect, _, err := dockerClient.ImageInspectWithRaw(context.TODO(), repoName)
h.AssertNil(t, err)

h.AssertEq(t, inspect.Os, daemonOS)
})
})

when("#SetOSVersion #SetArchitecture", func() {
var repoName = newTestImageName()

it.After(func() {
h.AssertNil(t, h.DockerRmi(dockerClient, repoName))
})

it("sets the os.version/arch", func() {
img, err := local.NewImage(repoName, dockerClient)
h.AssertNil(t, err)

err = img.SetOSVersion("1.2.3.4")
h.AssertNil(t, err)

err = img.SetArchitecture("arm64")
h.AssertNil(t, err)

h.AssertNil(t, img.Save())

inspect, _, err := dockerClient.ImageInspectWithRaw(context.TODO(), repoName)
h.AssertNil(t, err)

h.AssertEq(t, inspect.OsVersion, "1.2.3.4")
h.AssertEq(t, inspect.Architecture, "arm64")
})
})

when("#Rebase", func() {
when("image exists", func() {
var (
Expand Down
30 changes: 30 additions & 0 deletions remote/remote.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,36 @@ func (i *Image) SetCmd(cmd ...string) error {
return err
}

func (i *Image) SetOS(osVal string) error {
configFile, err := i.image.ConfigFile()
if err != nil {
return err
}
configFile.OS = osVal
i.image, err = mutate.ConfigFile(i.image, configFile)
return err
}

func (i *Image) SetOSVersion(osVersion string) error {
configFile, err := i.image.ConfigFile()
if err != nil {
return err
}
configFile.OSVersion = osVersion
i.image, err = mutate.ConfigFile(i.image, configFile)
return err
}

func (i *Image) SetArchitecture(architecture string) error {
configFile, err := i.image.ConfigFile()
if err != nil {
return err
}
configFile.Architecture = architecture
i.image, err = mutate.ConfigFile(i.image, configFile)
return err
}

func (i *Image) TopLayer() (string, error) {
all, err := i.image.Layers()
if err != nil {
Expand Down
21 changes: 21 additions & 0 deletions remote/remote_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,27 @@ func testImage(t *testing.T, when spec.G, it spec.S) {
})
})

when("#SetOS #SetOSVersion #SetArchitecture", func() {
it("sets the os/arch", func() {
img, err := remote.NewImage(repoName, authn.DefaultKeychain)
h.AssertNil(t, err)

err = img.SetOS("foobaros")
h.AssertNil(t, err)
err = img.SetOSVersion("1.2.3.4")
h.AssertNil(t, err)
err = img.SetArchitecture("arm64")
h.AssertNil(t, err)

h.AssertNil(t, img.Save())

configFile := h.FetchManifestImageConfigFile(t, repoName)
h.AssertEq(t, configFile.OS, "foobaros")
h.AssertEq(t, configFile.OSVersion, "1.2.3.4")
h.AssertEq(t, configFile.Architecture, "arm64")
})
})

when("#Rebase", func() {
when("image exists", func() {
var oldBase, newBase, oldTopLayerDiffID string
Expand Down