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

Add support for file based secret #1007

Closed
wants to merge 5 commits into from
Closed

Add support for file based secret #1007

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
40c7f36
Import spf13/cast package
hangyan May 16, 2018
0e503c9
Support file based secret
hangyan Jul 25, 2018
98fb348
Merge branch 'master' into secret-support
hangyan Aug 2, 2018
1adec75
Fix merge errors
hangyan Aug 2, 2018
7bd25b9
Use subPath for secret based volume
hangyan Oct 30, 2018
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
12 changes: 6 additions & 6 deletions docs/conversion.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ __Glossary:__
| deploy: replicas | - | - | ✓ | Deployment.Spec.Replicas / DeploymentConfig.Spec.Replicas | |
| deploy: placement | - | - | ✓ | Pod.Spec.NodeSelector | |
| deploy: update_config | - | - | n | | |
| deploy: resources | - | - | ✓ | Containers.Resources.Limits.Memory / Containers.Resources.Limits.CPU | Support for memory as well as cpu |
| deploy: resources | - | - | ✓ | Containers.Resources.Limits.Memory / Containers.Resources.Limits.CPU | Support for memory as well as cpu |
| deploy: restart_policy | - | - | ✓ | Pod generation | This generated a Pod, see the [user guide on restart](http://kompose.io/user-guide/#restart) |
| deploy: labels | - | - | n | | |
| devices | x | x | x | | Not supported within Kubernetes, See issue https://github.com/kubernetes/kubernetes/issues/5607 |
Expand All @@ -58,18 +58,18 @@ __Glossary:__
| labels | ✓ | ✓ | ✓ | Metadata.Annotations | |
| links | x | x | x | | All containers in the same pod are accessible in Kubernetes |
| logging | x | x | x | | Kubernetes has built-in logging support at the node-level |
| network_mode | x | x | x | | Kubernetes uses its own cluster networking |
| network_mode | x | x | x | | Kubernetes uses its own cluster networking |
| networks | x | x | x | | See `networks` key |
| networks: aliases | x | x | x | | See `networks` key |
| networks: addresses | x | x | x | | See `networks` key |
| pid | ✓ | ✓ | ✓ | Pod.Spec.HostPID | |
| ports | ✓ | ✓ | ✓ | Service.Spec.Ports | |
| ports: short-syntax | ✓ | ✓ | ✓ | Service.Spec.Ports | |
| ports: long-syntax | - | - | ✓ | Service.Spec.Ports | |
| secrets | - | - | n | | |
| secrets: short-syntax | - | - | n | | |
| secrets: long-syntax | - | - | n | | |
| security_opt | x | x | x | | Kubernetes uses its own container naming scheme |
| secrets | - | - | ✓ | Secret | External Secret is not Supported |
| secrets: short-syntax | - | - | ✓ | Secret | External Secret is not Supported |
| secrets: long-syntax | - | - | ✓ | Secret | External Secret is not Supported |
| security_opt | x | x | x | | Kubernetes uses its own container naming scheme |
| stop_grace_period | ✓ | ✓ | ✓ | Pod.Spec.TerminationGracePeriodSeconds | |
| stop_signal | x | x | x | | Not supported within Kubernetes. See issue https://github.com/kubernetes/kubernetes/issues/30051 |
| sysctls | n | n | n | | |
Expand Down
47 changes: 24 additions & 23 deletions glide.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions glide.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import:
- package: github.com/sirupsen/logrus
version: 1.0.3

- package: github.com/spf13/cast
version: v1.2.0

- package: golang.org/x/sys
version: ebfc5b4631820b793c9010c87fd8fef0f39eb082

Expand Down
7 changes: 5 additions & 2 deletions pkg/kobject/kobject.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ type KomposeObject struct {
// Transformer need to know origin format in order to tell user what tag is not supported in origin format
// as they can have different names. For example environment variables are called environment in compose but Env in bundle.
LoadedFrom string

Secrets map[string]dockerCliTypes.SecretConfig
}

// ConvertOptions holds all options that controls transformation process
Expand Down Expand Up @@ -107,8 +109,9 @@ type ServiceConfig struct {
Replicas int `compose:"replicas"`
GroupAdd []int64 `compose:"group_add"`
Volumes []Volumes `compose:""`
HealthChecks HealthCheck `compose:""`
Placement map[string]string `compose:""`
Secrets []dockerCliTypes.ServiceSecretConfig
HealthChecks HealthCheck `compose:""`
Placement map[string]string `compose:""`
//This is for long LONG SYNTAX link(https://docs.docker.com/compose/compose-file/#long-syntax)
Configs []dockerCliTypes.ServiceConfigObjConfig `compose:""`
//This is for SHORT SYNTAX link(https://docs.docker.com/compose/compose-file/#configs)
Expand Down
2 changes: 2 additions & 0 deletions pkg/loader/compose/v3.go
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ func dockerComposeToKomposeMapping(composeObject *types.Config) (kobject.Kompose
komposeObject := kobject.KomposeObject{
ServiceConfigs: make(map[string]kobject.ServiceConfig),
LoadedFrom: "compose",
Secrets: composeObject.Secrets,
}

// Step 2. Parse through the object and convert it to kobject.KomposeObject!
Expand Down Expand Up @@ -276,6 +277,7 @@ func dockerComposeToKomposeMapping(composeObject *types.Config) (kobject.Kompose
serviceConfig.Labels = composeServiceConfig.Labels
serviceConfig.HostName = composeServiceConfig.Hostname
serviceConfig.DomainName = composeServiceConfig.DomainName
serviceConfig.Secrets = composeServiceConfig.Secrets

//
// Deploy keys
Expand Down
11 changes: 11 additions & 0 deletions pkg/transformer/kubernetes/k8sutils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,18 @@ func GetEnvsFromFile(file string, opt kobject.ConvertOptions) (map[string]string
return envLoad, nil
}

// GetSecretDataFromFile load secret content data
func GetSecretDataFromFile(file string, opt kobject.ConvertOptions) ([]byte, error) {
composeDir, err := transformer.GetComposeFileDir(opt.InputFiles)
if err != nil {
return nil, errors.Wrap(err, "Unable to load file context")
}
fileLocation := path.Join(composeDir, file)
return ioutil.ReadFile(fileLocation)
}

// GetContentFromFile get content from file
// TODO(hang): merge these two functions
func GetContentFromFile(file string, opt kobject.ConvertOptions) (string, error) {
// Get the correct file context / directory
composeDir, err := transformer.GetComposeFileDir(opt.InputFiles)
Expand Down
101 changes: 101 additions & 0 deletions pkg/transformer/kubernetes/kubernetes.go
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ import (

"github.com/kubernetes/kompose/pkg/loader/compose"
"github.com/pkg/errors"
"github.com/spf13/cast"
"k8s.io/kubernetes/pkg/api/meta"
"k8s.io/kubernetes/pkg/labels"
"path/filepath"
Expand Down Expand Up @@ -372,6 +373,37 @@ func (k *Kubernetes) initIngress(name string, service kobject.ServiceConfig, por
return ingress
}

// CreateSecrets create secrets
func (k *Kubernetes) CreateSecrets(komposeObject kobject.KomposeObject) ([]*api.Secret, error) {
var objects []*api.Secret
for name, config := range komposeObject.Secrets {
if config.File != "" {
data, err := GetSecretDataFromFile(config.File, k.Opt)
if err != nil {
log.Fatal("unable to read secret from file: ", config.File)
return nil, err
}
secret := &api.Secret{
TypeMeta: unversioned.TypeMeta{
Kind: "Secret",
APIVersion: "v1",
},
ObjectMeta: api.ObjectMeta{
Name: name,
Labels: transformer.ConfigLabels(name),
},
Type: api.SecretTypeOpaque,
Data: map[string][]byte{name: data},
}
objects = append(objects, secret)
} else {
log.Warnf("External secrets %s is not currently supported - ignoring", name)
}
}
return objects, nil

}

// CreatePVC initializes PersistentVolumeClaim
func (k *Kubernetes) CreatePVC(name string, mode string, size string) (*api.PersistentVolumeClaim, error) {
volSize, err := resource.ParseQuantity(size)
Expand Down Expand Up @@ -516,6 +548,60 @@ func (k *Kubernetes) ConfigTmpfs(name string, service kobject.ServiceConfig) ([]
return volumeMounts, volumes
}

// ConfigSecretVolumes config volumes from secret.
// Link: https://docs.docker.com/compose/compose-file/#secrets
// In kubernetes' Secret resource, it has a data structure like a map[string]bytes, every key will act like the file name
// when mount to a container. This is the part that missing in compose. So we will create a single key secret from compose
// config and the key's name will be the secret's name, it's value is the file content.
// compose'secret can only be mounted at `/run/secrets`, so we will hardcoded this.
func (k *Kubernetes) ConfigSecretVolumes(name string, service kobject.ServiceConfig) ([]api.VolumeMount, []api.Volume) {
var volumeMounts []api.VolumeMount
var volumes []api.Volume
if len(service.Secrets) > 0 {
for _, secretConfig := range service.Secrets {
if secretConfig.UID != "" {
log.Warnf("Ignore pid in secrets for service: %s", name)
}
if secretConfig.GID != "" {
log.Warnf("Ignore gid in secrets for service: %s", name)
}

target := secretConfig.Target
if target == "" {
target = secretConfig.Source
}

volSource := api.VolumeSource{
Secret: &api.SecretVolumeSource{
SecretName: secretConfig.Source,
Items: []api.KeyToPath{{
Key: secretConfig.Source,
Path: target,
}},
},
}

if secretConfig.Mode != nil {
mode := cast.ToInt32(*secretConfig.Mode)
volSource.Secret.DefaultMode = &mode
}

vol := api.Volume{
Name: secretConfig.Source,
VolumeSource: volSource,
}
volumes = append(volumes, vol)

volMount := api.VolumeMount{
Name: vol.Name,
MountPath: "/run/secrets",
Copy link
Contributor

Choose a reason for hiding this comment

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

@hangyan
This shouldn't be MountPath: "/run/secrets/" + vol.Name, ???

Because without the MountPath targeting to the secret, it gives a error:

RunContainerError: failed to start container "f97d9e9354548939d15acae70758347538426fbb05c80f2a5cd316dc67e7bf58": 

Error response from daemon: oci runtime error: container_linux.go:247: starting container process caused "process_linux.go:359: 

container init caused \"rootfs_linux.go:54: 
mounting \\\"/opt/rke/var/lib/kubelet/pods/516c135b-bce8-11e8-8714-005056ae708d/volumes/kubernetes.io~secret/default-token-khnzc\\\" 
to rootfs \\\"/var/lib/docker/overlay/60fb5d9a4007bf8e1889898df737b275cb77431351b9168ab4d629fbcdf065b5/merged\\\" 
at \\\"/var/lib/docker/overlay/60fb5d9a4007bf8e1889898df737b275cb77431351b9168ab4d629fbcdf065b5/merged/run/secrets/kubernetes.io/serviceaccount\\\" 
caused \\\"mkdir /var/lib/docker/overlay/60fb5d9a4007bf8e1889898df737b275cb77431351b9168ab4d629fbcdf065b5/merged/run/secrets/kubernetes.io: read-only file system\\\"\""

Just like in this issue: kubernetes/kubernetes#65835

And concating the secret name, it work!

Copy link
Contributor

@jvitor83 jvitor83 Sep 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

With some research i discover that:

kubernetes

kubectl create secret generic my_secret --from-literal=my_secret_key_1=my_secret_value_1 --from-literal=my_secret_key_2=my_secret_value_2

it create a file /run/secrets/my_secret/my_secret_key_1 (with the content my_secret_value_1)

docker-compose/swarm

  • with short syntax:
...
    secrets:
      - my_secret
secrets:
  my_secret:
    file: my_secret.txt

it create a file /run/secrets/my_secret

don't allow to be compatible with kubernetes

  • with long syntax:
...
    secrets:
      - source: some_secret
        target: my_secret/my_secret_key
secrets:
  some_secret:
    file: my_secret.txt

it create a file /run/secrets/my_secret/my_secret_key

allow to be compatible with kubernetes

I believe that this PR:

  • shouldn't allow to use short-syntax, or if allow, to at least concat the secret name as directory (as my previous comment suggest to avoid the error).
  • only allow long-syntax if has at least inside one directory to be compatible with kubernetes, or at least concat the secret name as directory

Maybe concat the secret name as directory (by default) for the path can be the solution!?!

Copy link
Contributor Author

@hangyan hangyan Oct 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@jvitor83 Sorry about the late response. I have re-checked the document of kubernetes and docker. I have different opinion on this problem. kompose is meant to translate compose to kubernetes as precision as possible. Because docker mount the secret to /run/secrets/<secret-name>, so this path should be the same in kubernetes, not /run/secrets/<secret-name>/<key>. I believe the right choice is to use subPath ( you can see a example in the issues link you provide)

Copy link
Contributor

Choose a reason for hiding this comment

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

I get it that "kompose is meant to translate compose to kubernetes as precision as possible", but it didn't work with the k8s generated!
The generated mapping gives error (mentioned before) and the pod didn't start!

By my understanding, the generated files should work in first place to then keep the translation as precise as possible.

I guess we should remove the short syntax once it is incompatible and don't allow to get the file at the same path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue link you provide has a solution in the end:subPath. I think it's a better solution, i haven't test it out, but I think it should work

Copy link
Contributor

@jvitor83 jvitor83 Oct 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@hangyan
I am not understanding what you mean.

The thing is that this PR is generating:

        volumeMounts:
        - name: secret-name
          mountPath: "/run/secrets"

which gives the error reported.
The issue (kubernetes/kubernetes#65835) was solved not because it put the subPath, but because the directory was added to the mountPath. (I think the issue author have been mistaken)
If the kompose generate the k8s without the mountPath with a directory (with secret using short-syntax), it will give error on start. I have tested it with openjdk, dotnet and others. All gives the same error.

What you are suggesting to do?
If is to: "modify the code to add the subPath", it will continue to give the error.

I think we have to at least put some warning saying that secrets with short-syntax is not recommended and can gives error.

}
volumeMounts = append(volumeMounts, volMount)
}
}
return volumeMounts, volumes
}
Copy link
Member

Choose a reason for hiding this comment

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

Are you able to comment on some of the code above as to what's happening?


// ConfigVolumes configure the container volumes.
func (k *Kubernetes) ConfigVolumes(name string, service kobject.ServiceConfig) ([]api.VolumeMount, []api.Volume, []*api.PersistentVolumeClaim, error) {
volumeMounts := []api.VolumeMount{}
Expand All @@ -536,6 +622,11 @@ func (k *Kubernetes) ConfigVolumes(name string, service kobject.ServiceConfig) (
useHostPath = true
}

// config volumes from secret if present
secretsVolumeMounts, secretsVolumes := k.ConfigSecretVolumes(name, service)
Copy link
Member

Choose a reason for hiding this comment

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

Add comment above here explaining that we are adding secrets to volumes

volumeMounts = append(volumeMounts, secretsVolumeMounts...)
volumes = append(volumes, secretsVolumes...)

var count int
//iterating over array of `Vols` struct as it contains all necessary information about volumes
for _, volume := range service.Volumes {
Expand Down Expand Up @@ -806,6 +897,16 @@ func (k *Kubernetes) Transform(komposeObject kobject.KomposeObject, opt kobject.
// this will hold all the converted data
var allobjects []runtime.Object

if komposeObject.Secrets != nil {
secrets, err := k.CreateSecrets(komposeObject)
if err != nil {
return nil, errors.Wrapf(err, "Unable to create Secret resource")
}
for _, item := range secrets {
allobjects = append(allobjects, item)
}
}

sortedKeys := SortedKeys(komposeObject)
for _, name := range sortedKeys {
service := komposeObject.ServiceConfigs[name]
Expand Down
10 changes: 10 additions & 0 deletions pkg/transformer/openshift/openshift.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,16 @@ func (o *OpenShift) Transform(komposeObject kobject.KomposeObject, opt kobject.C
buildRepo := opt.BuildRepo
buildBranch := opt.BuildBranch

if komposeObject.Secrets != nil {
secrets, err := o.CreateSecrets(komposeObject)
if err != nil {
return nil, errors.Wrapf(err, "create secrets error")
}
for _, item := range secrets {
allobjects = append(allobjects, item)
}
}

sortedKeys := kubernetes.SortedKeys(komposeObject)
for _, name := range sortedKeys {
service := komposeObject.ServiceConfigs[name]
Expand Down
Loading