Merge pull request #302 from clcollins/OSD-25064_ocm-env-bug

BUGFIX: Make OCM environment inside of container ephemeral

Containerfile

@@ -193,6 +193,9 @@ ENV OCM_BACKPLANE_CONSOLE_PORT 9999
 EXPOSE $OCM_BACKPLANE_CONSOLE_PORT
 ENTRYPOINT ["/bin/bash"]
 
+# Create a directory for the ocm config file
+RUN mkdir -p /root/.config/ocm
+
 ### Final Minimal Image
 FROM base-update as ocm-container-minimal
 # ARG keeps the values from the final image

README.md

@@ -18,15 +18,12 @@ Thank you for your patience as we make this transition.
 
 First, download the latest release for your OS/Architecture: [https://github.com/openshift/ocm-container/releases](https://github.com/openshift/ocm-container/releases)
 
-Setup the base configuration, setting your preferred container engine (Podman or Docker) and OCM Token:
+Setup the base configuration, setting your preferred container engine (Podman or Docker):
 
 ```
 ocm-container configure set engine CONTAINER_ENGINE
-ocm-container configure set offline_access_token OCM_OFFLINE_ACCESS_TOKEN
 ```
 
-__Note:__ the OCM offline_access_token will be deprecated in the near future. OCM Container will be updated to handle this and assist in migrating your configuration.
-
 This is all that is required to get started with the basic setup, use the OCM cli, and log into clusters with OCM Backplane.
 
 ### Additional features
@@ -54,12 +51,36 @@ Running ocm-container can be done by executing the binary alone with no flags.
 ocm-container
 ```
 
+### Authentication
+
+OCM authentication defaults to using your OCM Config, first looking for the `OCM_CONFIG` environment variable.
+
+```
+OCM_CONFIG="~/.config/ocm/ocm.json.prod" ocm-container
+```
+
+If no `OCM_CONFIG` is specified, ocm-container will login to the environment proved in the `OCMC_OCM_URL` environment variable (prod, stage, int, prodgov) if set, then values provided by the `--ocm-url` flag.  If nothing is specified, the `--ocm-url` flag is set to "production" and that environment is used.
+
+```
+OCMC_OCM_URL=staging ocm-container
+
+# or
+
+ocm-container --ocm-url=staging
+```
+
+Upon login, OCM Container will copy a new ocm.json file to your `~/.config/ocm/` directory, in the format `ocm.json.ocm-container.$ocm_env`.  This file can be reused with the `OCM_CONFIG` environment variable in the future, if desired.
+
 Passing a cluster ID to the command with `--cluster-id` or `-C` will log you into that cluster after the container starts. This can be the cluster's OCM UUID, the OCM internal ID or the cluster's display name.
 
+### Cluster Login
+
 ```
 ocm-container --cluster-id CLUSTER_ID
 ```
 
+### Entrypoint
+
 By default, the container's Entrypoint is `/bin/bash`. You may also use the `--entrypoint=<command>` flag to change the container's Entrypoint as you would with a container engine.  The ocm-container binary also treats trailing non-flag arguments as container CMD arguments, again similar to how a container engine does.  For example, to execute the `ls` command as the Entrypoint and the flags `-lah` as the CMD, you can run:
 
 ```
@@ -74,6 +95,8 @@ You may also change the Entrypoint and CMD for use with an initial cluster ID fo
 ocm-container --entrypoint=ls --cluster-id CLUSTER_ID -- -lah
 ```
 
+### Container engine options
+
 Additional container engine arguments can be passed to the container using the `--launch-ops` flag.  These will be passed as-is to the engine, and are a best-effort support.  Some flags may conflict with ocm-container function.
 
 ```

pkg/engine/engine.go

@@ -93,6 +93,7 @@ func (e *Engine) Attach(c *Container) error {
 func (e *Engine) Copy(cpArgs ...string) (string, error) {
 	var args = []string{"cp"}
 	args = append(args, cpArgs...)
+	log.Debugf("executing command to copy files: %v %v\n", e.binary, args)
 	return e.exec(args...)
 }
 
@@ -138,7 +139,7 @@ func (e *Engine) Exec(c *Container, execArgs []string) (string, error) {
 	args = append(args, execArgs...)
 
 	if !e.dryRun {
-		log.Debug(fmt.Sprintf("executing command inside the running container: %v %v\n", e.binary, append([]string{e.engine}, args...)))
+		log.Debugf("executing command inside the running container: %v %v\n", e.binary, args)
 	}
 
 	out, err := e.exec(args...)
@@ -165,7 +166,7 @@ func (e *Engine) Start(c *Container, attach bool) error {
 	out, err := e.exec("start", c.ID)
 
 	// This is not log output; do not pass through a logger
-	log.Debug(fmt.Sprint("Exec output: " + out))
+	log.Debug("Exec output: " + out)
 
 	return err
 }

pkg/ocm/ocm.go

@@ -5,14 +5,15 @@ package ocm
 // creating the container
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
+	"path/filepath"
 
 	"github.com/openshift-online/ocm-cli/pkg/config"
 	sdk "github.com/openshift-online/ocm-sdk-go"
 	auth "github.com/openshift-online/ocm-sdk-go/authentication"
 	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
-	"github.com/openshift/ocm-container/pkg/engine"
 	"github.com/openshift/osdctl/pkg/utils"
 	log "github.com/sirupsen/logrus"
 )
@@ -23,9 +24,6 @@ const (
 	integrationURL   = "https://api.integration.openshift.com"
 	productionGovURL = "https://api.openshiftusgov.com"
 
-	ocmConfigDest      = "/root/.config/ocm/ocm.json"
-	ocmConfigMountOpts = "ro" // This should stay read-only, to keep the container from impacting the external environment
-
 	ocmContainerClientId = "ocm-cli"
 )
 
@@ -42,6 +40,13 @@ var (
 	}
 )
 
+var shortUrl = map[string]string{
+	productionURL:    "prod",
+	stagingURL:       "stage",
+	integrationURL:   "int",
+	productionGovURL: "prodgov",
+}
+
 var urlAliases = map[string]string{
 	"production":     productionURL,
 	"prod":           productionURL,
@@ -69,24 +74,14 @@ const (
 )
 
 type Config struct {
-	Env    map[string]string
-	Mounts []engine.VolumeMount
+	Env map[string]string
 }
 
-func New(ocmUrl string) (*Config, error) {
+func New(ocmcOcmUrl string) (*Config, error) {
 	c := &Config{}
 
 	c.Env = make(map[string]string)
 
-	// OCM URL is required by the OCM CLI inside the container
-	// otherwise the URL will be overridden by the saved OCM config
-	c.Env["OCM_URL"] = url(ocmUrl)
-	c.Env["OCMC_OCM_URL"] = url(ocmUrl)
-
-	if c.Env["OCMC_OCM_URL"] == "" {
-		return c, errInvalidOcmUrl
-	}
-
 	ocmConfig, err := config.Load()
 	if err != nil {
 		return c, err
@@ -96,6 +91,18 @@ func New(ocmUrl string) (*Config, error) {
 		ocmConfig = new(config.Config)
 	}
 
+	switch {
+	case os.Getenv("OCM_CONFIG") != "":
+		// ocmConfig.URL will already be set in this case
+		log.Debug("using OCM environment from $OCM_CONFIG")
+		if ocmcOcmUrl != "" {
+			log.Warnf("both $OCM_CONFIG and $OCMC_OCM_URL (or --ocm-url) are set; defaulting to $OCM_CONFIG for OCM environment")
+		}
+	default:
+		log.Info("using OCM environment from $OCMC_OCM_URL (or --ocm-url)")
+		ocmConfig.URL = url(ocmcOcmUrl)
+	}
+
 	armed, reason, err := ocmConfig.Armed()
 	if err != nil {
 		return c, fmt.Errorf("error checking OCM config arming: %s", err)
@@ -148,9 +155,6 @@ func New(ocmUrl string) (*Config, error) {
 	ocmConfig.ClientID = ocmContainerClientId
 	ocmConfig.TokenURL = sdk.DefaultTokenURL
 	ocmConfig.Scopes = defaultOcmScopes
-	// note - purposely not setting the ocmConfig.URL here
-	// to prevent overwriting the URL *outside* of the container
-	// The gateway is set by the OCM_URL env inside the container.  See above.
 
 	connection, err := ocmConfig.Connection()
 	if err != nil {
@@ -165,26 +169,18 @@ func New(ocmUrl string) (*Config, error) {
 	ocmConfig.AccessToken = accessToken
 	ocmConfig.RefreshToken = refreshToken
 
-	err = config.Save(ocmConfig)
+	// Note, we're saving our own copy of the OCM config here, to prevent overriding
+	ocmConfigLocation, err := save(ocmConfig)
 	if err != nil {
-		log.Warnf("non-fatal error saving OCM config: %s", err)
+		return c, fmt.Errorf("error saving copy of OCM config: %s", err)
 	}
 
-	ocmConfigLocation, err := config.Location()
-	if err != nil {
-		return c, fmt.Errorf("unable to identify OCM config location: %s", err)
-	}
+	c.Env["OCMC_EXTERNAL_OCM_CONFIG"] = ocmConfigLocation
+	c.Env["OCMC_INTERNAL_OCM_CONFIG"] = "/root/.config/ocm/ocm.json"
 
-	ocmVolume := engine.VolumeMount{
-		Source:       ocmConfigLocation,
-		Destination:  ocmConfigDest,
-		MountOptions: ocmConfigMountOpts,
-	}
-
-	_, err = os.Stat(ocmVolume.Source)
-	if !os.IsNotExist(err) {
-
-		c.Mounts = append(c.Mounts, ocmVolume)
+	_, err = os.Stat(ocmConfigLocation)
+	if os.IsNotExist(err) {
+		return c, fmt.Errorf("OCM config file does not exist: %s", ocmConfigLocation)
 	}
 
 	return c, nil
@@ -196,6 +192,12 @@ func url(s string) string {
 	return urlAliases[s]
 }
 
+// alias takes a string in the form of an OCM_URL, and returns
+// a short alias
+func alias(s string) string {
+	return shortUrl[s]
+}
+
 func NewClient() (*sdk.Connection, error) {
 	ocmClient, err := utils.CreateConnection()
 	if err != nil {
@@ -225,3 +227,30 @@ func GetClusterId(ocmClient *sdk.Connection, key string) (string, error) {
 
 	return cluster.ID(), err
 }
+
+// save takes a *config.Config and saves it to a file alongside the existing OCM config
+// The path is the same as the existing OCM config, but the filename follows the convention:
+// ocm.json.ocm-container.$ocm_env
+func save(cfg *config.Config) (string, error) {
+	file, err := config.Location()
+	if err != nil {
+		return "", err
+	}
+	dir := filepath.Dir(file)
+	err = os.MkdirAll(dir, os.FileMode(0755))
+	if err != nil {
+		return "", fmt.Errorf("can't create directory %s: %v", dir, err)
+	}
+	data, err := json.MarshalIndent(cfg, "", "  ")
+	if err != nil {
+		return "", fmt.Errorf("can't marshal config: %v", err)
+	}
+
+	cachedConfig := dir + "/ocm.json.ocm-container." + alias(cfg.URL)
+
+	err = os.WriteFile(cachedConfig, data, 0600)
+	if err != nil {
+		return "", fmt.Errorf("can't write file '%s': %v", file, err)
+	}
+	return cachedConfig, nil
+}

pkg/ocmcontainer/ocmcontainer.go

@@ -131,7 +131,6 @@ func New(cmd *cobra.Command, args []string) (*ocmContainer, error) {
 	}
 
 	maps.Copy(c.Envs, ocmConfig.Env)
-	c.Volumes = append(c.Volumes, ocmConfig.Mounts...)
 
 	// OCM-Container optional features follow:
 
@@ -275,6 +274,21 @@ func New(cmd *cobra.Command, args []string) (*ocmContainer, error) {
 
 	log.Printf("container created with ID: %v\n", o.container.ID)
 
+	log.Debugf(
+		"copying ocm config into container: %s - %s\n",
+		ocmConfig.Env["OCMC_EXTERNAL_OCM_CONFIG"],
+		ocmConfig.Env["OCMC_INTERNAL_OCM_CONFIG"],
+	)
+
+	ocmConfigSource := ocmConfig.Env["OCMC_EXTERNAL_OCM_CONFIG"]
+	ocmConfigDest := fmt.Sprintf("%s:%s", o.container.ID, ocmConfig.Env["OCMC_INTERNAL_OCM_CONFIG"])
+
+	out, err := o.Copy(ocmConfigSource, ocmConfigDest)
+	log.Debug(out)
+	if err != nil {
+		return o, err
+	}
+
 	return o, nil
 }
 
@@ -532,15 +546,15 @@ func (o *ocmContainer) Exec(args []string) (string, error) {
 
 // Copy takes a source and destination (optionally with a [container]: prefixed)
 // and executes a container engine "cp" command with those as arguments
-func (o *ocmContainer) Copy(source, destination string) error {
+func (o *ocmContainer) Copy(source, destination string) (string, error) {
 	s := filepath.Clean(source)
 	d := filepath.Clean(destination)
 
-	args := fmt.Sprintf("%s:%s", s, d)
+	args := []string{s, d}
 
-	o.engine.Copy("cp", args)
+	out, err := o.engine.Copy(args...)
 
-	return nil
+	return out, err
 }
 
 func (o *ocmContainer) Inspect(query string) (string, error) {

utils/bashrc.d/14-kube-ps1.bashrc

@@ -1,5 +1,5 @@
 # shellcheck shell=bash
-export PS1="[\W {\[\033[1;32m\]\${OCM_URL}\[\033[0m\]} \$(kube_ps1)]\$ "
+export PS1="[\W {\[\033[1;32m\]\$(ocm config get url)\[\033[0m\]} \$(kube_ps1)]\$ "
 export KUBE_PS1_BINARY=oc
 export KUBE_PS1_CLUSTER_FUNCTION=cluster_function
-export KUBE_PS1_SYMBOL_ENABLE=false
+export KUBE_PS1_SYMBOL_ENABLE=false