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

Add option --unsetenv to remove default environment variables #12100

Merged
merged 1 commit into from
Nov 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions cmd/podman/common/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,20 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions,
)
_ = cmd.RegisterFlagCompletionFunc(envFlagName, completion.AutocompleteNone)

unsetenvFlagName := "unsetenv"
createFlags.StringArrayVar(
&cf.UnsetEnv,
unsetenvFlagName, []string{},
"Unset environment default variables in container",
)
_ = cmd.RegisterFlagCompletionFunc(unsetenvFlagName, completion.AutocompleteNone)

createFlags.BoolVar(
&cf.UnsetEnvAll,
"unsetenv-all", false,
"Unset all default environment variables in container",
)

if !registry.IsRemote() {
createFlags.BoolVar(
&cf.EnvHost,
Expand Down
2 changes: 2 additions & 0 deletions cmd/podman/common/create_opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,8 @@ func ContainerCreateToContainerCLIOpts(cc handlers.CreateContainerConfig, rtc *c
Systemd: "true", // podman default
TmpFS: parsedTmp,
TTY: cc.Config.Tty,
UnsetEnv: cc.UnsetEnv,
UnsetEnvAll: cc.UnsetEnvAll,
User: cc.Config.User,
UserNS: string(cc.HostConfig.UsernsMode),
UTS: string(cc.HostConfig.UTSMode),
Expand Down
12 changes: 12 additions & 0 deletions docs/source/markdown/podman-create.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -1047,6 +1047,18 @@ Remote connections use local containers.conf for defaults
Set the umask inside the container. Defaults to `0022`.
Remote connections use local containers.conf for defaults

#### **--unsetenv**=*env*

Unset default environment variables for the container. Default environment
variables include variables provided natively by Podman, environment variables
configured by the image, and environment variables from containers.conf.

#### **--unsetenv-all**=*true|false*

Unset all default environment variables for the container. Default environment
variables include variables provided natively by Podman, environment variables
configured by the image, and environment variables from containers.conf.

#### **--uidmap**=*container_uid*:*from_uid*:*amount*

Run the container in a new user namespace using the supplied mapping. This
Expand Down
12 changes: 12 additions & 0 deletions docs/source/markdown/podman-run.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,18 @@ Remote connections use local containers.conf for defaults
Set the umask inside the container. Defaults to `0022`.
Remote connections use local containers.conf for defaults

#### **--unsetenv**=*env*

Unset default environment variables for the container. Default environment
variables include variables provided natively by Podman, environment variables
configured by the image, and environment variables from containers.conf.

#### **--unsetenv-all**=*true|false*

Unset all default environment variables for the container. Default environment
variables include variables provided natively by Podman, environment variables
configured by the image, and environment variables from containers.conf.

TomSweeneyRedHat marked this conversation as resolved.
Show resolved Hide resolved
#### **--uidmap**=*container_uid*:*from_uid*:*amount*

Run the container in a new user namespace using the supplied mapping. This
Expand Down
3 changes: 1 addition & 2 deletions libpod/container_inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,7 @@ func (c *Container) generateInspectContainerConfig(spec *spec.Spec) *define.Insp
ctrConfig.User = c.config.User
if spec.Process != nil {
ctrConfig.Tty = spec.Process.Terminal
ctrConfig.Env = []string{}
ctrConfig.Env = append(ctrConfig.Env, spec.Process.Env...)
ctrConfig.Env = append([]string{}, spec.Process.Env...)
ctrConfig.WorkingDir = spec.Process.Cwd
}

Expand Down
12 changes: 0 additions & 12 deletions libpod/container_internal_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -709,18 +709,6 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
g.AddAnnotation(annotations.ContainerManager, annotations.ContainerManagerLibpod)
}

// Only add container environment variable if not already present
foundContainerEnv := false
for _, env := range g.Config.Process.Env {
if strings.HasPrefix(env, "container=") {
foundContainerEnv = true
break
}
}
if !foundContainerEnv {
g.AddProcessEnv("container", "libpod")
}

cgroupPath, err := c.getOCICgroupPath()
if err != nil {
return nil, err
Expand Down
2 changes: 2 additions & 0 deletions pkg/api/handlers/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ type CreateContainerConfig struct {
dockerContainer.Config // desired container configuration
HostConfig dockerContainer.HostConfig // host dependent configuration for container
NetworkingConfig dockerNetwork.NetworkingConfig // network configuration for container
UnsetEnv []string // unset specified default environment variables
UnsetEnvAll bool // unset all default environment variables
}

// swagger:model IDResponse
Expand Down
2 changes: 2 additions & 0 deletions pkg/domain/entities/pods.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ type ContainerCreateOptions struct {
TTY bool
Timezone string
Umask string
UnsetEnv []string
UnsetEnvAll bool
UIDMap []string
Ulimit []string
User string
Expand Down
12 changes: 8 additions & 4 deletions pkg/specgen/generate/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,6 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
if err != nil {
return nil, errors.Wrap(err, "error parsing fields in containers.conf")
}
if defaultEnvs["container"] == "" {
defaultEnvs["container"] = "podman"
Copy link
Member

Choose a reason for hiding this comment

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

Where is this getting set now? You've removed every instance of container=podman I can find.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is set in pkg/env/DefaultEnvVariables()

}
var envs map[string]string

// Image Environment defaults
Expand All @@ -101,9 +98,16 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
if err != nil {
return nil, errors.Wrap(err, "Env fields from image failed to parse")
}
defaultEnvs = envLib.Join(defaultEnvs, envs)
defaultEnvs = envLib.Join(envLib.DefaultEnvVariables(), envLib.Join(defaultEnvs, envs))
}

for _, e := range s.UnsetEnv {
delete(defaultEnvs, e)
}

if s.UnsetEnvAll {
defaultEnvs = make(map[string]string)
}
// First transform the os env into a map. We need it for the labels later in
// any case.
osEnv, err := envLib.ParseSlice(os.Environ())
Expand Down
2 changes: 1 addition & 1 deletion pkg/specgen/generate/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
for key, val := range s.Annotations {
g.AddAnnotation(key, val)
}
g.AddProcessEnv("container", "podman")

g.Config.Linux.Resources = s.ResourceLimits
// Devices
Expand Down Expand Up @@ -332,6 +331,7 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt

BlockAccessToKernelFilesystems(s.Privileged, s.PidNS.IsHost(), s.Mask, s.Unmask, &g)

g.ClearProcessEnv()
Copy link
Member

Choose a reason for hiding this comment

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

This seems dangerous, you're clearing all the default environment variables?

Copy link
Member Author

Choose a reason for hiding this comment

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

All the defaults set in runtime-tools, Which sets TERM and PATH. We set both of those and currently override what runtime-tools sets.

for name, val := range s.Env {
g.AddProcessEnv(name, val)
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/specgen/specgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ type ContainerBasicConfig struct {
// The execution domain system allows Linux to provide limited support
// for binaries compiled under other UNIX-like operating systems.
Personality *spec.LinuxPersonality `json:"personality,omitempty"`
// UnsetEnv unsets the specified default environment variables from the image or from buildin or containers.conf
// Optional.
UnsetEnv []string `json:"unsetenv,omitempty"`
// UnsetEnvAll unsetall default environment variables from the image or from buildin or containers.conf
// UnsetEnvAll unsets all default environment variables from the image or from buildin
// Optional.
UnsetEnvAll bool `json:"unsetenvall,omitempty"`
}

// ContainerStorageConfig contains information on the storage configuration of a
Expand Down
2 changes: 2 additions & 0 deletions pkg/specgenutil/specgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,8 @@ func FillOutSpecGen(s *specgen.SpecGenerator, c *entities.ContainerCreateOptions
s.Umask = c.Umask
s.PidFile = c.PidFile
s.Volatile = c.Rm
s.UnsetEnv = c.UnsetEnv
s.UnsetEnvAll = c.UnsetEnvAll

// Initcontainers
s.InitContainerType = c.InitContainerType
Expand Down
22 changes: 22 additions & 0 deletions test/system/030-run.bats
Original file line number Diff line number Diff line change
Expand Up @@ -736,4 +736,26 @@ EOF
is "$output" "$random_1" "output matches STDIN"
}

@test "podman run defaultenv" {
run_podman run --rm $IMAGE printenv
is "$output" ".*TERM=xterm" "output matches TERM"
is "$output" ".*container=podman" "output matches container=podman"

run_podman run --unsetenv=TERM --rm $IMAGE printenv
is "$output" ".*container=podman" "output matches container=podman"
run grep TERM <<<$output
is "$output" "" "unwanted TERM environment variable despite --unsetenv=TERM"

run_podman run --unsetenv-all --rm $IMAGE /bin/printenv
run grep TERM <<<$output
is "$output" "" "unwanted TERM environment variable despite --unsetenv-all"
run grep container <<<$output
is "$output" "" "unwanted container environment variable despite --unsetenv-all"
run grep PATH <<<$output
is "$output" "" "unwanted PATH environment variable despite --unsetenv-all"

run_podman run --unsetenv-all --env TERM=abc --rm $IMAGE /bin/printenv
is "$output" ".*TERM=abc" "missing TERM environment variable despite TERM being set on commandline"
}

# vim: filetype=sh
8 changes: 6 additions & 2 deletions test/system/250-systemd.bats
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,14 @@ function check_listen_env() {
if is_remote; then
is "$output" "$stdenv" "LISTEN Environment did not pass: $context"
else
is "$output" "$stdenv
out=$(for o in $output; do echo $o; done| sort)
Copy link
Member

Choose a reason for hiding this comment

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

If for any reason you need to resubmit, could I suggest this instead?

    out=$(sort <<<"$output")

Reason: if there's ever an environment variable whose value includes a space (unlikely), the for loop will print each space-separated token on a new line.

std=$(echo "$stdenv
LISTEN_PID=1
LISTEN_FDS=1
LISTEN_FDNAMES=listen_fdnames" "LISTEN Environment passed: $context"
LISTEN_FDNAMES=listen_fdnames" | sort)
echo "<$out>"
echo "<$std>"
Comment on lines +182 to +183
Copy link
Member

Choose a reason for hiding this comment

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

You probably don't need these, since the is will display them on failure.

is "$out" "$std" "LISTEN Environment passed: $context"
fi
}

Expand Down