From acb8046f840d4189d143ca2572263179deb0d17c Mon Sep 17 00:00:00 2001 From: AhmedGrati <48932084+AhmedGrati@users.noreply.github.com> Date: Sat, 19 Nov 2022 03:35:33 +0100 Subject: [PATCH] Fix environment variables interpolation (#1524) --- pkg/kobject/kobject.go | 1 + pkg/loader/compose/v3.go | 5 +- pkg/transformer/kubernetes/k8sutils.go | 31 ++++- pkg/transformer/kubernetes/k8sutils_test.go | 32 +++++ pkg/transformer/kubernetes/kubernetes.go | 18 +-- pkg/transformer/kubernetes/podspec.go | 14 ++ pkg/utils/docker/image.go | 1 + pkg/utils/docker/tag.go | 1 + script/test/cmd/tests_new.sh | 10 +- .../envvars-interpolation/docker-compose.yaml | 13 ++ .../envvars-interpolation/output-k8s.json | 60 +++++++++ .../envvars-interpolation/output-os.json | 120 ++++++++++++++++++ 12 files changed, 288 insertions(+), 18 deletions(-) create mode 100644 script/test/fixtures/envvars-interpolation/docker-compose.yaml create mode 100644 script/test/fixtures/envvars-interpolation/output-k8s.json create mode 100644 script/test/fixtures/envvars-interpolation/output-os.json diff --git a/pkg/kobject/kobject.go b/pkg/kobject/kobject.go index c5913d6c4..9a85fd1c5 100644 --- a/pkg/kobject/kobject.go +++ b/pkg/kobject/kobject.go @@ -91,6 +91,7 @@ func (opt *ConvertOptions) IsPodController() bool { return opt.IsDeploymentFlag || opt.IsDaemonSetFlag || opt.IsReplicationControllerFlag || opt.Controller != "" } +// ServiceConfigGroup holds an array of a ServiceConfig objects. type ServiceConfigGroup []ServiceConfig // ServiceConfig holds the basic struct of a container diff --git a/pkg/loader/compose/v3.go b/pkg/loader/compose/v3.go index 2e218e76a..1bd7399b2 100644 --- a/pkg/loader/compose/v3.go +++ b/pkg/loader/compose/v3.go @@ -105,7 +105,9 @@ func parseV3(files []string) (kobject.KomposeObject, error) { // We load it in order to retrieve the parsed output configuration! // This will output a github.com/docker/cli ServiceConfig // Which is similar to our version of ServiceConfig - currentConfig, err := loader.Load(configDetails) + currentConfig, err := loader.Load(configDetails, func(options *loader.Options) { + options.SkipInterpolation = true + }) if err != nil { return kobject.KomposeObject{}, err } @@ -129,7 +131,6 @@ func parseV3(files []string) (kobject.KomposeObject, error) { if err != nil { return kobject.KomposeObject{}, err } - return komposeObject, nil } diff --git a/pkg/transformer/kubernetes/k8sutils.go b/pkg/transformer/kubernetes/k8sutils.go index 534584e13..a1b7823e5 100644 --- a/pkg/transformer/kubernetes/k8sutils.go +++ b/pkg/transformer/kubernetes/k8sutils.go @@ -25,6 +25,7 @@ import ( "path" "path/filepath" "reflect" + "regexp" "sort" "strconv" "strings" @@ -382,6 +383,7 @@ func (k *Kubernetes) initSvcObject(name string, service kobject.ServiceConfig, p return svc } +// CreateLBService creates a k8s Load Balancer Service func (k *Kubernetes) CreateLBService(name string, service kobject.ServiceConfig) []*api.Service { var svcs []*api.Service tcpPorts, udpPorts := k.ConfigLBServicePorts(service) @@ -442,6 +444,8 @@ func (k *Kubernetes) CreateHeadlessService(name string, service kobject.ServiceC return svc } + +// UpdateKubernetesObjectsMultipleContainers method updates the kubernetes objects with the necessary data func (k *Kubernetes) UpdateKubernetesObjectsMultipleContainers(name string, service kobject.ServiceConfig, objects *[]runtime.Object, podSpec PodSpec) error { // Configure annotations annotations := transformer.ConfigAnnotations(service) @@ -524,7 +528,7 @@ func (k *Kubernetes) UpdateKubernetesObjects(name string, service kobject.Servic template.Spec.Containers[0].Name = GetContainerName(service) template.Spec.Containers[0].Env = envs template.Spec.Containers[0].Command = service.Command - template.Spec.Containers[0].Args = service.Args + template.Spec.Containers[0].Args = GetContainerArgs(service) template.Spec.Containers[0].WorkingDir = service.WorkingDir template.Spec.Containers[0].VolumeMounts = append(template.Spec.Containers[0].VolumeMounts, volumesMount...) template.Spec.Containers[0].Stdin = service.Stdin @@ -594,18 +598,18 @@ func (k *Kubernetes) UpdateKubernetesObjects(name string, service kobject.Servic template.ObjectMeta.Labels = transformer.ConfigLabelsWithNetwork(name, service.Network) // Configure the image pull policy - if policy, err := GetImagePullPolicy(name, service.ImagePullPolicy); err != nil { + policy, err := GetImagePullPolicy(name, service.ImagePullPolicy) + if err != nil { return err - } else { - template.Spec.Containers[0].ImagePullPolicy = policy } + template.Spec.Containers[0].ImagePullPolicy = policy // Configure the container restart policy. - if restart, err := GetRestartPolicy(name, service.Restart); err != nil { + restart, err := GetRestartPolicy(name, service.Restart) + if err != nil { return err - } else { - template.Spec.RestartPolicy = restart } + template.Spec.RestartPolicy = restart // Configure hostname/domain_name settings if service.HostName != "" { @@ -887,6 +891,7 @@ func FormatContainerName(name string) string { return name } +// GetContainerName returns the name of the container, from the service config object func GetContainerName(service kobject.ServiceConfig) string { name := service.Name if len(service.ContainerName) > 0 { @@ -899,3 +904,15 @@ func GetContainerName(service kobject.ServiceConfig) string { func FormatResourceName(name string) string { return strings.ToLower(strings.Replace(name, "_", "-", -1)) } + +// GetContainerArgs update the interpolation of env variables if exists. +// example: [curl, $PROTOCOL://$DOMAIN] => [curl, $(PROTOCOL)://$(DOMAIN)] +func GetContainerArgs(service kobject.ServiceConfig) []string { + var args []string + re := regexp.MustCompile(`\$([a-zA-Z0-9]*)`) + for _, arg := range service.Args { + arg = re.ReplaceAllString(arg, `$($1)`) + args = append(args, arg) + } + return args +} diff --git a/pkg/transformer/kubernetes/k8sutils_test.go b/pkg/transformer/kubernetes/k8sutils_test.go index 22aae955e..e9358905a 100644 --- a/pkg/transformer/kubernetes/k8sutils_test.go +++ b/pkg/transformer/kubernetes/k8sutils_test.go @@ -597,3 +597,35 @@ func TestCreateServiceWithSpecialName(t *testing.T) { } } } + +func TestArgsInterpolation(t *testing.T) { + // An example service + service := kobject.ServiceConfig{ + ContainerName: "name", + Image: "image", + Environment: []kobject.EnvVar{{Name: "PROTOCOL", Value: "https"}, {Name: "DOMAIN", Value: "google.com"}}, + Port: []kobject.Ports{{HostPort: 123, ContainerPort: 456, Protocol: string(corev1.ProtocolTCP)}}, + Command: []string{"curl"}, + Args: []string{"$PROTOCOL://$DOMAIN/"}, + } + + // An example object generated via k8s runtime.Objects() + komposeObject := kobject.KomposeObject{ + ServiceConfigs: map[string]kobject.ServiceConfig{"app": service}, + } + k := Kubernetes{} + objects, err := k.Transform(komposeObject, kobject.ConvertOptions{CreateD: true, Replicas: 3}) + if err != nil { + t.Error(errors.Wrap(err, "k.Transform failed")) + } + + expectedArgs := []string{"$(PROTOCOL)://$(DOMAIN)/"} + for _, obj := range objects { + if deployment, ok := obj.(*appsv1.Deployment); ok { + args := deployment.Spec.Template.Spec.Containers[0].Args[0] + if args != expectedArgs[0] { + t.Errorf("Expected args %v upon conversion, actual %v", expectedArgs, args) + } + } + } +} diff --git a/pkg/transformer/kubernetes/kubernetes.go b/pkg/transformer/kubernetes/kubernetes.go index 5de64d459..296356725 100644 --- a/pkg/transformer/kubernetes/kubernetes.go +++ b/pkg/transformer/kubernetes/kubernetes.go @@ -58,6 +58,7 @@ type Kubernetes struct { // PVCRequestSize (Persistent Volume Claim) has default size const PVCRequestSize = "100Mi" +// ValidVolumeSet has the different types of valid volumes var ValidVolumeSet = map[string]struct{}{"emptyDir": {}, "hostPath": {}, "configMap": {}, "persistentVolumeClaim": {}} const ( @@ -432,6 +433,7 @@ func (k *Kubernetes) InitDS(name string, service kobject.ServiceConfig) *appsv1. return ds } +// InitSS method initialize a stateful set func (k *Kubernetes) InitSS(name string, service kobject.ServiceConfig, replicas int) *appsv1.StatefulSet { var podSpec api.PodSpec if len(service.Configs) > 0 { @@ -634,6 +636,7 @@ func ConfigPorts(service kobject.ServiceConfig) []api.ContainerPort { return ports } +// ConfigLBServicePorts method configure the ports of the k8s Load Balancer Service func (k *Kubernetes) ConfigLBServicePorts(service kobject.ServiceConfig) ([]api.ServicePort, []api.ServicePort) { var tcpPorts []api.ServicePort var udpPorts []api.ServicePort @@ -954,16 +957,15 @@ func (k *Kubernetes) ConfigVolumes(name string, service kobject.ServiceConfig) ( volsource = source } else if useConfigMap { log.Debugf("Use configmap volume") - - if cm, err := k.IntiConfigMapFromFileOrDir(name, volumeName, volume.Host, service); err != nil { + cm, err := k.IntiConfigMapFromFileOrDir(name, volumeName, volume.Host, service) + if err != nil { return nil, nil, nil, nil, err - } else { - cms = append(cms, cm) - volsource = k.ConfigConfigMapVolumeSource(volumeName, volume.Container, cm) + } + cms = append(cms, cm) + volsource = k.ConfigConfigMapVolumeSource(volumeName, volume.Container, cm) - if useSubPathMount(cm) { - volMount.SubPath = volsource.ConfigMap.Items[0].Path - } + if useSubPathMount(cm) { + volMount.SubPath = volsource.ConfigMap.Items[0].Path } } else { volsource = k.ConfigPVCVolumeSource(volumeName, readonly) diff --git a/pkg/transformer/kubernetes/podspec.go b/pkg/transformer/kubernetes/podspec.go index 7531b3eb4..d52378419 100644 --- a/pkg/transformer/kubernetes/podspec.go +++ b/pkg/transformer/kubernetes/podspec.go @@ -13,12 +13,15 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" ) +// PodSpec holds the spec of k8s pod. type PodSpec struct { api.PodSpec } +// PodSpecOption holds the function to apply on a PodSpec type PodSpecOption func(*PodSpec) +// AddContainer method is responsible for adding a new container to a k8s Pod. func AddContainer(service kobject.ServiceConfig, opt kobject.ConvertOptions) PodSpecOption { return func(podSpec *PodSpec) { name := GetContainerName(service) @@ -50,6 +53,7 @@ func AddContainer(service kobject.ServiceConfig, opt kobject.ConvertOptions) Pod } } +// TerminationGracePeriodSeconds method is responsible for attributing the grace period seconds option to a pod func TerminationGracePeriodSeconds(name string, service kobject.ServiceConfig) PodSpecOption { return func(podSpec *PodSpec) { var err error @@ -156,6 +160,7 @@ func SecurityContext(name string, service kobject.ServiceConfig) PodSpecOption { } } +// SetVolumeNames method return a set of volume names func SetVolumeNames(volumes []api.Volume) mapset.Set { set := mapset.NewSet() for _, volume := range volumes { @@ -164,6 +169,7 @@ func SetVolumeNames(volumes []api.Volume) mapset.Set { return set } +// SetVolumes method returns a method that adds the volumes to the pod spec func SetVolumes(volumes []api.Volume) PodSpecOption { return func(podSpec *PodSpec) { volumesSet := SetVolumeNames(volumes) @@ -179,6 +185,7 @@ func SetVolumes(volumes []api.Volume) PodSpecOption { } } +// SetVolumeMountPaths method returns a set of volumes mount path func SetVolumeMountPaths(volumesMount []api.VolumeMount) mapset.Set { set := mapset.NewSet() for _, volumeMount := range volumesMount { @@ -188,6 +195,7 @@ func SetVolumeMountPaths(volumesMount []api.VolumeMount) mapset.Set { return set } +// SetVolumeMounts returns a function which adds the volume mounts option to the pod spec func SetVolumeMounts(volumesMount []api.VolumeMount) PodSpecOption { return func(podSpec *PodSpec) { volumesMountSet := SetVolumeMountPaths(volumesMount) @@ -242,6 +250,7 @@ func RestartPolicy(name string, service kobject.ServiceConfig) PodSpecOption { } } +// HostName configure the host name of a pod func HostName(service kobject.ServiceConfig) PodSpecOption { return func(podSpec *PodSpec) { // Configure hostname/domain_name settings @@ -251,6 +260,7 @@ func HostName(service kobject.ServiceConfig) PodSpecOption { } } +// DomainName configure the domain name of a pod func DomainName(service kobject.ServiceConfig) PodSpecOption { return func(podSpec *PodSpec) { if service.DomainName != "" { @@ -299,18 +309,21 @@ func configProbe(healthCheck kobject.HealthCheck) *api.Probe { return &probe } +// ServiceAccountName is responsible for setting the service account name to the pod spec func ServiceAccountName(serviceAccountName string) PodSpecOption { return func(podSpec *PodSpec) { podSpec.ServiceAccountName = serviceAccountName } } +// TopologySpreadConstraints is responsible for setting the topology spread constraints to the pod spec func TopologySpreadConstraints(service kobject.ServiceConfig) PodSpecOption { return func(podSpec *PodSpec) { podSpec.TopologySpreadConstraints = ConfigTopologySpreadConstraints(service) } } +// Append is responsible for adding the pod spec options to the particular pod func (podSpec *PodSpec) Append(ops ...PodSpecOption) *PodSpec { for _, option := range ops { option(podSpec) @@ -318,6 +331,7 @@ func (podSpec *PodSpec) Append(ops ...PodSpecOption) *PodSpec { return podSpec } +// Get is responsible for returning the pod spec of a particular pod func (podSpec *PodSpec) Get() api.PodSpec { return podSpec.PodSpec } diff --git a/pkg/utils/docker/image.go b/pkg/utils/docker/image.go index edd1ec48d..f9cf4c682 100644 --- a/pkg/utils/docker/image.go +++ b/pkg/utils/docker/image.go @@ -33,6 +33,7 @@ type Image struct { Remote string // the image's remote identifier. (ie: registry/name[:tag]) } +// NewImageFromParsed method returns the docker image from the docker parser reference func NewImageFromParsed(parsed *dockerparser.Reference) Image { return Image{ Name: parsed.Name(), diff --git a/pkg/utils/docker/tag.go b/pkg/utils/docker/tag.go index 6c0e6f07d..482617b71 100644 --- a/pkg/utils/docker/tag.go +++ b/pkg/utils/docker/tag.go @@ -27,6 +27,7 @@ type Tag struct { Client dockerlib.Client } +// TagImage function is responsible for tagging the docker image func (c *Tag) TagImage(image Image) error { options := dockerlib.TagImageOptions{ Tag: image.Tag, diff --git a/script/test/cmd/tests_new.sh b/script/test/cmd/tests_new.sh index f63e5ad79..34a101857 100755 --- a/script/test/cmd/tests_new.sh +++ b/script/test/cmd/tests_new.sh @@ -202,4 +202,12 @@ os_cmd="kompose --provider=openshift -f $KOMPOSE_ROOT/script/test/fixtures/mul k8s_output="$KOMPOSE_ROOT/script/test/fixtures/multiple-type-volumes/output-k8s.json" os_output="$KOMPOSE_ROOT/script/test/fixtures/multiple-type-volumes/output-os.json" convert::expect_success_and_warning "$k8s_cmd" "$k8s_output" -convert::expect_success "$os_cmd" "$os_output" \ No newline at end of file +convert::expect_success "$os_cmd" "$os_output" + +# Test environment variables interpolation +k8s_cmd="kompose -f $KOMPOSE_ROOT/script/test/fixtures/envvars-interpolation/docker-compose.yaml convert --stdout -j --with-kompose-annotation=false" +os_cmd="kompose --provider=openshift -f $KOMPOSE_ROOT/script/test/fixtures/envvars-interpolation/docker-compose.yaml convert --stdout -j --with-kompose-annotation=false" +k8s_output="$KOMPOSE_ROOT/script/test/fixtures/envvars-interpolation/output-k8s.json" +os_output="$KOMPOSE_ROOT/script/test/fixtures/envvars-interpolation/output-os.json" +convert::expect_success_and_warning "$k8s_cmd" "$k8s_output" +convert::expect_success "$os_cmd" "$os_output" diff --git a/script/test/fixtures/envvars-interpolation/docker-compose.yaml b/script/test/fixtures/envvars-interpolation/docker-compose.yaml new file mode 100644 index 000000000..6623f589b --- /dev/null +++ b/script/test/fixtures/envvars-interpolation/docker-compose.yaml @@ -0,0 +1,13 @@ +version: '3.5' + +services: + myservice: + image: alpine + environment: + PROTOCOL: 'https' + DOMAIN: 'google.com' + command: + [ + 'curl', + '$PROTOCOL://$DOMAIN/', + ] diff --git a/script/test/fixtures/envvars-interpolation/output-k8s.json b/script/test/fixtures/envvars-interpolation/output-k8s.json new file mode 100644 index 000000000..6ee8b10bf --- /dev/null +++ b/script/test/fixtures/envvars-interpolation/output-k8s.json @@ -0,0 +1,60 @@ +{ + "kind": "List", + "apiVersion": "v1", + "metadata": {}, + "items": [ + { + "kind": "Deployment", + "apiVersion": "apps/v1", + "metadata": { + "name": "myservice", + "creationTimestamp": null, + "labels": { + "io.kompose.service": "myservice" + } + }, + "spec": { + "replicas": 1, + "selector": { + "matchLabels": { + "io.kompose.service": "myservice" + } + }, + "template": { + "metadata": { + "creationTimestamp": null, + "labels": { + "io.kompose.service": "myservice" + } + }, + "spec": { + "containers": [ + { + "name": "myservice", + "image": "alpine", + "args": [ + "curl", + "$(PROTOCOL)://$(DOMAIN)/" + ], + "env": [ + { + "name": "DOMAIN", + "value": "google.com" + }, + { + "name": "PROTOCOL", + "value": "https" + } + ], + "resources": {} + } + ], + "restartPolicy": "Always" + } + }, + "strategy": {} + }, + "status": {} + } + ] +} diff --git a/script/test/fixtures/envvars-interpolation/output-os.json b/script/test/fixtures/envvars-interpolation/output-os.json new file mode 100644 index 000000000..8f7796f80 --- /dev/null +++ b/script/test/fixtures/envvars-interpolation/output-os.json @@ -0,0 +1,120 @@ +{ + "kind": "List", + "apiVersion": "v1", + "metadata": {}, + "items": [ + { + "kind": "DeploymentConfig", + "apiVersion": "v1", + "metadata": { + "name": "myservice", + "creationTimestamp": null, + "labels": { + "io.kompose.service": "myservice" + } + }, + "spec": { + "strategy": { + "resources": {} + }, + "triggers": [ + { + "type": "ConfigChange" + }, + { + "type": "ImageChange", + "imageChangeParams": { + "automatic": true, + "containerNames": [ + "myservice" + ], + "from": { + "kind": "ImageStreamTag", + "name": "myservice:latest" + } + } + } + ], + "replicas": 1, + "test": false, + "selector": { + "io.kompose.service": "myservice" + }, + "template": { + "metadata": { + "creationTimestamp": null, + "labels": { + "io.kompose.service": "myservice" + } + }, + "spec": { + "containers": [ + { + "name": "myservice", + "image": " ", + "args": [ + "curl", + "$(PROTOCOL)://$(DOMAIN)/" + ], + "env": [ + { + "name": "DOMAIN", + "value": "google.com" + }, + { + "name": "PROTOCOL", + "value": "https" + } + ], + "resources": {} + } + ], + "restartPolicy": "Always" + } + } + }, + "status": { + "latestVersion": 0, + "observedGeneration": 0, + "replicas": 0, + "updatedReplicas": 0, + "availableReplicas": 0, + "unavailableReplicas": 0 + } + }, + { + "kind": "ImageStream", + "apiVersion": "v1", + "metadata": { + "name": "myservice", + "creationTimestamp": null, + "labels": { + "io.kompose.service": "myservice" + } + }, + "spec": { + "lookupPolicy": { + "local": false + }, + "tags": [ + { + "name": "latest", + "annotations": null, + "from": { + "kind": "DockerImage", + "name": "alpine" + }, + "generation": null, + "importPolicy": {}, + "referencePolicy": { + "type": "" + } + } + ] + }, + "status": { + "dockerImageRepository": "" + } + } + ] +}