1
0
mirror of https://github.com/containers/buildah.git synced 2025-07-31 15:24:26 +03:00

Drop copyStringSlice() and copyStringStringMap()

Use slices.Clone() and maps.Clone() instead of our own non-generic
functions.  We have to be more careful in a couple of places where we
set items in maps which aren't unconditionally initialized.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
This commit is contained in:
Nalin Dahyabhai
2024-06-03 14:44:47 -04:00
parent 509b4b5b51
commit a42019d614
11 changed files with 61 additions and 57 deletions

View File

@ -25,6 +25,7 @@ import (
"github.com/containers/storage/pkg/stringid" "github.com/containers/storage/pkg/stringid"
digest "github.com/opencontainers/go-digest" digest "github.com/opencontainers/go-digest"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
"golang.org/x/exp/maps"
) )
const ( const (
@ -325,7 +326,7 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
logrus.Debugf("committing image with reference %q is allowed by policy", transports.ImageName(dest)) logrus.Debugf("committing image with reference %q is allowed by policy", transports.ImageName(dest))
// If we need to scan the rootfs, do it now. // If we need to scan the rootfs, do it now.
options.ExtraImageContent = copyStringStringMap(options.ExtraImageContent) options.ExtraImageContent = maps.Clone(options.ExtraImageContent)
var extraImageContent, extraLocalContent map[string]string var extraImageContent, extraLocalContent map[string]string
if len(options.SBOMScanOptions) != 0 { if len(options.SBOMScanOptions) != 0 {
var scansDirectory string var scansDirectory string
@ -339,9 +340,14 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
} }
}() }()
} }
for k, v := range extraImageContent { if len(extraImageContent) > 0 {
if _, set := options.ExtraImageContent[k]; !set { if options.ExtraImageContent == nil {
options.ExtraImageContent[k] = v options.ExtraImageContent = make(map[string]string, len(extraImageContent))
}
for k, v := range extraImageContent {
if _, set := options.ExtraImageContent[k]; !set {
options.ExtraImageContent[k] = v
}
} }
} }
} }

View File

@ -19,6 +19,7 @@ import (
"github.com/containers/storage/pkg/stringid" "github.com/containers/storage/pkg/stringid"
ociv1 "github.com/opencontainers/image-spec/specs-go/v1" ociv1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
"golang.org/x/exp/maps"
"golang.org/x/exp/slices" "golang.org/x/exp/slices"
) )
@ -91,8 +92,13 @@ func (b *Builder) initConfig(ctx context.Context, img types.Image, sys *types.Sy
if err := json.Unmarshal(b.Manifest, &v1Manifest); err != nil { if err := json.Unmarshal(b.Manifest, &v1Manifest); err != nil {
return fmt.Errorf("parsing OCI manifest %q: %w", string(b.Manifest), err) return fmt.Errorf("parsing OCI manifest %q: %w", string(b.Manifest), err)
} }
for k, v := range v1Manifest.Annotations { if len(v1Manifest.Annotations) > 0 {
b.ImageAnnotations[k] = v if b.ImageAnnotations == nil {
b.ImageAnnotations = make(map[string]string, len(v1Manifest.Annotations))
}
for k, v := range v1Manifest.Annotations {
b.ImageAnnotations[k] = v
}
} }
} }
} }
@ -158,7 +164,7 @@ func (b *Builder) setupLogger() {
// Annotations returns a set of key-value pairs from the image's manifest. // Annotations returns a set of key-value pairs from the image's manifest.
func (b *Builder) Annotations() map[string]string { func (b *Builder) Annotations() map[string]string {
return copyStringStringMap(b.ImageAnnotations) return maps.Clone(b.ImageAnnotations)
} }
// SetAnnotation adds or overwrites a key's value from the image's manifest. // SetAnnotation adds or overwrites a key's value from the image's manifest.
@ -180,7 +186,7 @@ func (b *Builder) UnsetAnnotation(key string) {
// ClearAnnotations removes all keys and their values from the image's // ClearAnnotations removes all keys and their values from the image's
// manifest. // manifest.
func (b *Builder) ClearAnnotations() { func (b *Builder) ClearAnnotations() {
b.ImageAnnotations = map[string]string{} b.ImageAnnotations = nil
} }
// CreatedBy returns a description of how this image was built. // CreatedBy returns a description of how this image was built.
@ -223,7 +229,7 @@ func (b *Builder) SetOSVersion(version string) {
// OSFeatures returns a list of OS features which the container, or a container // OSFeatures returns a list of OS features which the container, or a container
// built using an image built from this container, depends on the OS supplying. // built using an image built from this container, depends on the OS supplying.
func (b *Builder) OSFeatures() []string { func (b *Builder) OSFeatures() []string {
return copyStringSlice(b.OCIv1.OSFeatures) return slices.Clone(b.OCIv1.OSFeatures)
} }
// SetOSFeature adds a feature of the OS which the container, or a container // SetOSFeature adds a feature of the OS which the container, or a container
@ -327,7 +333,7 @@ func (b *Builder) SetUser(spec string) {
// OnBuild returns the OnBuild value from the container. // OnBuild returns the OnBuild value from the container.
func (b *Builder) OnBuild() []string { func (b *Builder) OnBuild() []string {
return copyStringSlice(b.Docker.Config.OnBuild) return slices.Clone(b.Docker.Config.OnBuild)
} }
// ClearOnBuild removes all values from the OnBuild structure // ClearOnBuild removes all values from the OnBuild structure
@ -363,7 +369,7 @@ func (b *Builder) SetWorkDir(there string) {
// Shell returns the default shell for running commands in the // Shell returns the default shell for running commands in the
// container, or in a container built using an image built from this container. // container, or in a container built using an image built from this container.
func (b *Builder) Shell() []string { func (b *Builder) Shell() []string {
return copyStringSlice(b.Docker.Config.Shell) return slices.Clone(b.Docker.Config.Shell)
} }
// SetShell sets the default shell for running // SetShell sets the default shell for running
@ -376,13 +382,13 @@ func (b *Builder) SetShell(shell []string) {
b.Logger.Warnf("SHELL is not supported for OCI image format, %s will be ignored. Must use `docker` format", shell) b.Logger.Warnf("SHELL is not supported for OCI image format, %s will be ignored. Must use `docker` format", shell)
} }
b.Docker.Config.Shell = copyStringSlice(shell) b.Docker.Config.Shell = slices.Clone(shell)
} }
// Env returns a list of key-value pairs to be set when running commands in the // Env returns a list of key-value pairs to be set when running commands in the
// container, or in a container built using an image built from this container. // container, or in a container built using an image built from this container.
func (b *Builder) Env() []string { func (b *Builder) Env() []string {
return copyStringSlice(b.OCIv1.Config.Env) return slices.Clone(b.OCIv1.Config.Env)
} }
// SetEnv adds or overwrites a value to the set of environment strings which // SetEnv adds or overwrites a value to the set of environment strings which
@ -432,22 +438,22 @@ func (b *Builder) ClearEnv() {
// set, to use when running a container built from an image built from this // set, to use when running a container built from an image built from this
// container. // container.
func (b *Builder) Cmd() []string { func (b *Builder) Cmd() []string {
return copyStringSlice(b.OCIv1.Config.Cmd) return slices.Clone(b.OCIv1.Config.Cmd)
} }
// SetCmd sets the default command, or command parameters if an Entrypoint is // SetCmd sets the default command, or command parameters if an Entrypoint is
// set, to use when running a container built from an image built from this // set, to use when running a container built from an image built from this
// container. // container.
func (b *Builder) SetCmd(cmd []string) { func (b *Builder) SetCmd(cmd []string) {
b.OCIv1.Config.Cmd = copyStringSlice(cmd) b.OCIv1.Config.Cmd = slices.Clone(cmd)
b.Docker.Config.Cmd = copyStringSlice(cmd) b.Docker.Config.Cmd = slices.Clone(cmd)
} }
// Entrypoint returns the command to be run for containers built from images // Entrypoint returns the command to be run for containers built from images
// built from this container. // built from this container.
func (b *Builder) Entrypoint() []string { func (b *Builder) Entrypoint() []string {
if len(b.OCIv1.Config.Entrypoint) > 0 { if len(b.OCIv1.Config.Entrypoint) > 0 {
return copyStringSlice(b.OCIv1.Config.Entrypoint) return slices.Clone(b.OCIv1.Config.Entrypoint)
} }
return nil return nil
} }
@ -455,14 +461,14 @@ func (b *Builder) Entrypoint() []string {
// SetEntrypoint sets the command to be run for in containers built from images // SetEntrypoint sets the command to be run for in containers built from images
// built from this container. // built from this container.
func (b *Builder) SetEntrypoint(ep []string) { func (b *Builder) SetEntrypoint(ep []string) {
b.OCIv1.Config.Entrypoint = copyStringSlice(ep) b.OCIv1.Config.Entrypoint = slices.Clone(ep)
b.Docker.Config.Entrypoint = copyStringSlice(ep) b.Docker.Config.Entrypoint = slices.Clone(ep)
} }
// Labels returns a set of key-value pairs from the image's runtime // Labels returns a set of key-value pairs from the image's runtime
// configuration. // configuration.
func (b *Builder) Labels() map[string]string { func (b *Builder) Labels() map[string]string {
return copyStringStringMap(b.OCIv1.Config.Labels) return maps.Clone(b.OCIv1.Config.Labels)
} }
// SetLabel adds or overwrites a key's value from the image's runtime // SetLabel adds or overwrites a key's value from the image's runtime
@ -669,7 +675,7 @@ func (b *Builder) Healthcheck() *docker.HealthConfig {
return nil return nil
} }
return &docker.HealthConfig{ return &docker.HealthConfig{
Test: copyStringSlice(b.Docker.Config.Healthcheck.Test), Test: slices.Clone(b.Docker.Config.Healthcheck.Test),
Interval: b.Docker.Config.Healthcheck.Interval, Interval: b.Docker.Config.Healthcheck.Interval,
Timeout: b.Docker.Config.Healthcheck.Timeout, Timeout: b.Docker.Config.Healthcheck.Timeout,
StartPeriod: b.Docker.Config.Healthcheck.StartPeriod, StartPeriod: b.Docker.Config.Healthcheck.StartPeriod,
@ -690,7 +696,7 @@ func (b *Builder) SetHealthcheck(config *docker.HealthConfig) {
b.Logger.Warnf("HEALTHCHECK is not supported for OCI image format and will be ignored. Must use `docker` format") b.Logger.Warnf("HEALTHCHECK is not supported for OCI image format and will be ignored. Must use `docker` format")
} }
b.Docker.Config.Healthcheck = &docker.HealthConfig{ b.Docker.Config.Healthcheck = &docker.HealthConfig{
Test: copyStringSlice(config.Test), Test: slices.Clone(config.Test),
Interval: config.Interval, Interval: config.Interval,
Timeout: config.Timeout, Timeout: config.Timeout,
StartPeriod: config.StartPeriod, StartPeriod: config.StartPeriod,

View File

@ -32,6 +32,7 @@ import (
specs "github.com/opencontainers/image-spec/specs-go" specs "github.com/opencontainers/image-spec/specs-go"
v1 "github.com/opencontainers/image-spec/specs-go/v1" v1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
"golang.org/x/exp/maps"
) )
const ( const (
@ -1102,7 +1103,7 @@ func (b *Builder) makeContainerImageRef(options CommitOptions) (*containerImageR
postEmptyLayers: b.AppendedEmptyLayers, postEmptyLayers: b.AppendedEmptyLayers,
overrideChanges: options.OverrideChanges, overrideChanges: options.OverrideChanges,
overrideConfig: options.OverrideConfig, overrideConfig: options.OverrideConfig,
extraImageContent: copyStringStringMap(options.ExtraImageContent), extraImageContent: maps.Clone(options.ExtraImageContent),
} }
return ref, nil return ref, nil
} }

View File

@ -70,6 +70,9 @@ func BuildDockerfiles(ctx context.Context, store storage.Store, options define.B
if options.CommonBuildOpts == nil { if options.CommonBuildOpts == nil {
options.CommonBuildOpts = &define.CommonBuildOptions{} options.CommonBuildOpts = &define.CommonBuildOptions{}
} }
if options.Args == nil {
options.Args = make(map[string]string)
}
if err := parse.Volumes(options.CommonBuildOpts.Volumes); err != nil { if err := parse.Volumes(options.CommonBuildOpts.Volumes); err != nil {
return "", nil, fmt.Errorf("validating volumes: %w", err) return "", nil, fmt.Errorf("validating volumes: %w", err)
} }

View File

@ -35,6 +35,7 @@ import (
"github.com/openshift/imagebuilder" "github.com/openshift/imagebuilder"
"github.com/openshift/imagebuilder/dockerfile/parser" "github.com/openshift/imagebuilder/dockerfile/parser"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
"golang.org/x/exp/slices"
"golang.org/x/sync/semaphore" "golang.org/x/sync/semaphore"
) )
@ -267,9 +268,9 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o
defaultMountsFilePath: options.DefaultMountsFilePath, defaultMountsFilePath: options.DefaultMountsFilePath,
iidfile: options.IIDFile, iidfile: options.IIDFile,
squash: options.Squash, squash: options.Squash,
labels: append([]string{}, options.Labels...), labels: slices.Clone(options.Labels),
layerLabels: append([]string{}, options.LayerLabels...), layerLabels: slices.Clone(options.LayerLabels),
annotations: append([]string{}, options.Annotations...), annotations: slices.Clone(options.Annotations),
layers: options.Layers, layers: options.Layers,
noHostname: options.CommonBuildOpts.NoHostname, noHostname: options.CommonBuildOpts.NoHostname,
noHosts: options.CommonBuildOpts.NoHosts, noHosts: options.CommonBuildOpts.NoHosts,
@ -306,12 +307,12 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o
secrets: secrets, secrets: secrets,
sshsources: sshsources, sshsources: sshsources,
logPrefix: logPrefix, logPrefix: logPrefix,
unsetEnvs: append([]string{}, options.UnsetEnvs...), unsetEnvs: slices.Clone(options.UnsetEnvs),
unsetLabels: append([]string{}, options.UnsetLabels...), unsetLabels: slices.Clone(options.UnsetLabels),
buildOutput: options.BuildOutput, buildOutput: options.BuildOutput,
osVersion: options.OSVersion, osVersion: options.OSVersion,
osFeatures: append([]string{}, options.OSFeatures...), osFeatures: slices.Clone(options.OSFeatures),
envs: append([]string{}, options.Envs...), envs: slices.Clone(options.Envs),
confidentialWorkload: options.ConfidentialWorkload, confidentialWorkload: options.ConfidentialWorkload,
sbomScanOptions: options.SBOMScanOptions, sbomScanOptions: options.SBOMScanOptions,
cdiConfigDir: options.CDIConfigDir, cdiConfigDir: options.CDIConfigDir,

View File

@ -97,7 +97,6 @@ func importBuilderDataFromImage(ctx context.Context, store storage.Store, system
FromImageDigest: imageDigest, FromImageDigest: imageDigest,
Container: containerName, Container: containerName,
ContainerID: containerID, ContainerID: containerID,
ImageAnnotations: map[string]string{},
ImageCreatedBy: "", ImageCreatedBy: "",
NamespaceOptions: defaultNamespaceOptions, NamespaceOptions: defaultNamespaceOptions,
IDMappingOptions: define.IDMappingOptions{ IDMappingOptions: define.IDMappingOptions{

View File

@ -1,6 +1,8 @@
package config package config
import ( import (
"slices"
"github.com/containers/image/v5/manifest" "github.com/containers/image/v5/manifest"
dockerclient "github.com/fsouza/go-dockerclient" dockerclient "github.com/fsouza/go-dockerclient"
) )
@ -41,17 +43,17 @@ func Schema2ConfigFromGoDockerclientConfig(config *dockerclient.Config) *manifes
Tty: config.Tty, Tty: config.Tty,
OpenStdin: config.OpenStdin, OpenStdin: config.OpenStdin,
StdinOnce: config.StdinOnce, StdinOnce: config.StdinOnce,
Env: append([]string{}, config.Env...), Env: slices.Clone(config.Env),
Cmd: append([]string{}, config.Cmd...), Cmd: slices.Clone(config.Cmd),
Healthcheck: overrideHealthCheck, Healthcheck: overrideHealthCheck,
ArgsEscaped: config.ArgsEscaped, ArgsEscaped: config.ArgsEscaped,
Image: config.Image, Image: config.Image,
Volumes: volumes, Volumes: volumes,
WorkingDir: config.WorkingDir, WorkingDir: config.WorkingDir,
Entrypoint: append([]string{}, config.Entrypoint...), Entrypoint: slices.Clone(config.Entrypoint),
NetworkDisabled: config.NetworkDisabled, NetworkDisabled: config.NetworkDisabled,
MacAddress: config.MacAddress, MacAddress: config.MacAddress,
OnBuild: append([]string{}, config.OnBuild...), OnBuild: slices.Clone(config.OnBuild),
Labels: labels, Labels: labels,
StopSignal: config.StopSignal, StopSignal: config.StopSignal,
Shell: config.Shell, Shell: config.Shell,

6
new.go
View File

@ -21,6 +21,8 @@ import (
v1 "github.com/opencontainers/image-spec/specs-go/v1" v1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/openshift/imagebuilder" "github.com/openshift/imagebuilder"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
"golang.org/x/exp/maps"
"golang.org/x/exp/slices"
) )
const ( const (
@ -313,10 +315,10 @@ func newBuilder(ctx context.Context, store storage.Store, options BuilderOptions
UIDMap: uidmap, UIDMap: uidmap,
GIDMap: gidmap, GIDMap: gidmap,
}, },
Capabilities: copyStringSlice(options.Capabilities), Capabilities: slices.Clone(options.Capabilities),
CommonBuildOpts: options.CommonBuildOpts, CommonBuildOpts: options.CommonBuildOpts,
TopLayer: topLayer, TopLayer: topLayer,
Args: copyStringStringMap(options.Args), Args: maps.Clone(options.Args),
Format: options.Format, Format: options.Format,
TempVolumes: map[string]bool{}, TempVolumes: map[string]bool{},
Devices: options.Devices, Devices: options.Devices,

View File

@ -53,6 +53,7 @@ import (
"github.com/opencontainers/runtime-tools/generate" "github.com/opencontainers/runtime-tools/generate"
"github.com/opencontainers/selinux/go-selinux/label" "github.com/opencontainers/selinux/go-selinux/label"
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
"golang.org/x/exp/slices"
"golang.org/x/sys/unix" "golang.org/x/sys/unix"
"golang.org/x/term" "golang.org/x/term"
) )
@ -540,7 +541,7 @@ func runUsingRuntime(options RunOptions, configureNetwork bool, moreCreateArgs [
} }
} }
runtimeArgs := options.Args[:] runtimeArgs := slices.Clone(options.Args)
if options.CgroupManager == config.SystemdCgroupsManager { if options.CgroupManager == config.SystemdCgroupsManager {
runtimeArgs = append(runtimeArgs, "--systemd-cgroup") runtimeArgs = append(runtimeArgs, "--systemd-cgroup")
} }

View File

@ -51,6 +51,7 @@ import (
"github.com/sirupsen/logrus" "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"golang.org/x/exp/slices"
) )
const ( const (
@ -1239,11 +1240,7 @@ func compareJSON(a, b map[string]interface{}, skip []string) (missKeys, leftKeys
} }
} }
replace := func(slice []string) []string { return slices.Clone(missKeys), slices.Clone(leftKeys), slices.Clone(diffKeys), isSame
return append([]string{}, slice...)
}
return replace(missKeys), replace(leftKeys), replace(diffKeys), isSame
} }
// configCompareResult summarizes the output of compareJSON for display // configCompareResult summarizes the output of compareJSON for display

14
util.go
View File

@ -28,20 +28,6 @@ func InitReexec() bool {
return reexec.Init() return reexec.Init()
} }
func copyStringStringMap(m map[string]string) map[string]string {
n := map[string]string{}
for k, v := range m {
n[k] = v
}
return n
}
func copyStringSlice(s []string) []string {
t := make([]string, len(s))
copy(t, s)
return t
}
func copyHistory(history []v1.History) []v1.History { func copyHistory(history []v1.History) []v1.History {
if len(history) == 0 { if len(history) == 0 {
return nil return nil