Skip to content

Commit

Permalink
change storage of ssh pem from configmap to secret for ssh plugin
Browse files Browse the repository at this point in the history
  • Loading branch information
sivanzcw committed Dec 13, 2019
1 parent 13e343f commit 519f2a9
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 44 deletions.
3 changes: 3 additions & 0 deletions installer/helm/chart/volcano/templates/controllers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ rules:
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["get", "list", "watch", "create", "delete", "update"]
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get", "list", "watch", "create", "delete", "update"]
- apiGroups: ["scheduling.incubator.k8s.io", "scheduling.sigs.dev"]
resources: ["podgroups", "queues", "queues/status"]
verbs: ["get", "list", "watch", "create", "delete", "update"]
Expand Down
3 changes: 3 additions & 0 deletions installer/volcano-development.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,9 @@ rules:
- apiGroups: [""]
resources: ["configmaps"]
verbs: ["get", "list", "watch", "create", "delete", "update"]
- apiGroups: [""]
resources: ["secrets"]
verbs: ["get", "list", "watch", "create", "delete", "update"]
- apiGroups: ["scheduling.incubator.k8s.io", "scheduling.sigs.dev"]
resources: ["podgroups", "queues", "queues/status"]
verbs: ["get", "list", "watch", "create", "delete", "update"]
Expand Down
28 changes: 28 additions & 0 deletions pkg/apis/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,24 @@ func CreateConfigMapIfNotExist(job *vcbatch.Job, kubeClients kubernetes.Interfac
return nil
}

// CreateSecret create secret
func CreateSecret(job *vcbatch.Job, kubeClients kubernetes.Interface, data map[string][]byte, secretName string) error {
secret := &v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: job.Namespace,
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(job, JobKind),
},
},
Data: data,
}

_, err := kubeClients.CoreV1().Secrets(job.Namespace).Create(secret)

return err
}

// DeleteConfigmap deletes the config map resource
func DeleteConfigmap(job *vcbatch.Job, kubeClients kubernetes.Interface, cmName string) error {
if _, err := kubeClients.CoreV1().ConfigMaps(job.Namespace).Get(cmName, metav1.GetOptions{}); err != nil {
Expand All @@ -145,6 +163,16 @@ func DeleteConfigmap(job *vcbatch.Job, kubeClients kubernetes.Interface, cmName
return nil
}

// DeleteSecret delete secret
func DeleteSecret(job *vcbatch.Job, kubeClients kubernetes.Interface, secretName string) error {
err := kubeClients.CoreV1().Secrets(job.Namespace).Delete(secretName, nil)
if err != nil && true == apierrors.IsNotFound(err) {
return nil
}

return err
}

// GeneratePodgroupName generate podgroup name of normal pod
func GeneratePodgroupName(pod *v1.Pod) string {
pgName := vcbatch.PodgroupNamePrefix
Expand Down
22 changes: 12 additions & 10 deletions pkg/controllers/job/job_controller_actions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func TestKillJobFunc(t *testing.T) {
JobInfo *apis.JobInfo
Services []v1.Service
ConfigMaps []v1.ConfigMap
Secrets []v1.Secret
Pods map[string]*v1.Pod
Plugins []string
ExpextVal error
Expand All @@ -52,6 +53,7 @@ func TestKillJobFunc(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "job1",
Namespace: namespace,
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
},
},
PodGroup: &schedulingv1alpha2.PodGroup{
Expand Down Expand Up @@ -80,10 +82,10 @@ func TestKillJobFunc(t *testing.T) {
},
},
},
ConfigMaps: []v1.ConfigMap{
Secrets: []v1.Secret{
{
ObjectMeta: metav1.ObjectMeta{
Name: "job1-ssh",
Name: "job1-e7f18111-1cec-11ea-b688-fa163ec79500-ssh",
Namespace: namespace,
},
},
Expand All @@ -110,10 +112,10 @@ func TestKillJobFunc(t *testing.T) {
}
}

for _, configMap := range testcase.ConfigMaps {
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Create(&configMap)
for _, secret := range testcase.Secrets {
_, err := fakeController.kubeClient.CoreV1().Secrets(namespace).Create(&secret)
if err != nil {
t.Error("Error While Creating ConfigMaps")
t.Error("Error While Creating Secret.")
}
}

Expand Down Expand Up @@ -142,26 +144,26 @@ func TestKillJobFunc(t *testing.T) {

err = fakeController.killJob(testcase.JobInfo, testcase.PodRetainPhase, testcase.UpdateStatus)
if err != nil {
t.Errorf("Case %d (%s): expected: No Error, but got error %s", i, testcase.Name, err.Error())
t.Errorf("Case %d (%s): expected: No Error, but got error %v.", i, testcase.Name, err)
}

for _, plugin := range testcase.Plugins {

if plugin == "svc" {
_, err = fakeController.kubeClient.CoreV1().Services(namespace).Get(testcase.Job.Name, metav1.GetOptions{})
if err == nil {
t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted.", i, testcase.Name)
}
}

if plugin == "ssh" {
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-ssh"), metav1.GetOptions{})
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(
fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh"), metav1.GetOptions{})
if err == nil {
t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
t.Errorf("Case %d (%s): expected: Secret to be deleted, but not deleted.", i, testcase.Name)
}
}
}

})
}
}
Expand Down
23 changes: 15 additions & 8 deletions pkg/controllers/job/job_controller_plugins_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func TestPluginOnPodCreate(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "Job1",
Namespace: namespace,
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
},
},
Pod: buildPod(namespace, "pod1", v1.PodPending, nil),
Expand All @@ -66,6 +67,7 @@ func TestPluginOnPodCreate(t *testing.T) {
Job: &batch.Job{
ObjectMeta: metav1.ObjectMeta{
Name: "Job1",
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
},
},
Pod: buildPod(namespace, "pod1", v1.PodPending, nil),
Expand Down Expand Up @@ -124,7 +126,7 @@ func TestPluginOnPodCreate(t *testing.T) {
}
exist := false
for _, volume := range container.VolumeMounts {
if volume.Name == fmt.Sprint(testcase.Job.Name, "-ssh") {
if volume.Name == fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh") {
exist = true
}
}
Expand Down Expand Up @@ -153,6 +155,7 @@ func TestPluginOnJobAdd(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "job1",
Namespace: namespace,
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
},
},
Plugins: []string{"svc", "ssh", "env"},
Expand All @@ -163,6 +166,7 @@ func TestPluginOnJobAdd(t *testing.T) {
Job: &batch.Job{
ObjectMeta: metav1.ObjectMeta{
Name: "Job1",
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
},
},
Plugins: []string{"new"},
Expand Down Expand Up @@ -202,9 +206,10 @@ func TestPluginOnJobAdd(t *testing.T) {
}

if plugin == "ssh" {
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-ssh"), metav1.GetOptions{})
_, err := fakeController.kubeClient.CoreV1().Secrets(namespace).Get(
fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh"), metav1.GetOptions{})
if err != nil {
t.Errorf("Case %d (%s): expected: ConfigMap to be created, but not created because of error %s", i, testcase.Name, err.Error())
t.Errorf("Case %d (%s): expected: Secret to be created, but not created because of error %s", i, testcase.Name, err.Error())
}
}

Expand Down Expand Up @@ -233,6 +238,7 @@ func TestPluginOnJobDelete(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "job1",
Namespace: namespace,
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
},
},
Plugins: []string{"svc", "ssh", "env"},
Expand All @@ -243,6 +249,7 @@ func TestPluginOnJobDelete(t *testing.T) {
Job: &batch.Job{
ObjectMeta: metav1.ObjectMeta{
Name: "Job1",
UID: "e7f18111-1cec-11ea-b688-fa163ec79500",
},
},
Plugins: []string{"new"},
Expand Down Expand Up @@ -272,23 +279,23 @@ func TestPluginOnJobDelete(t *testing.T) {
if plugin == "svc" {
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-svc"), metav1.GetOptions{})
if err == nil {
t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted.", i, testcase.Name)
}

_, err = fakeController.kubeClient.CoreV1().Services(namespace).Get(testcase.Job.Name, metav1.GetOptions{})
if err == nil {
t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
t.Errorf("Case %d (%s): expected: Service to be deleted, but not deleted.", i, testcase.Name)
}
}

if plugin == "ssh" {
_, err := fakeController.kubeClient.CoreV1().ConfigMaps(namespace).Get(fmt.Sprint(testcase.Job.Name, "-ssh"), metav1.GetOptions{})
_, err := fakeController.kubeClient.CoreV1().Secrets(namespace).Get(
fmt.Sprintf("%s-%s-%s", testcase.Job.Name, testcase.Job.UID, "ssh"), metav1.GetOptions{})
if err == nil {
t.Errorf("Case %d (%s): expected: ConfigMap to be deleted, but not deleted because of error %s", i, testcase.Name, err.Error())
t.Errorf("Case %d (%s): expected: secret to be deleted, but not deleted.", i, testcase.Name)
}
}
}
})

}
}
40 changes: 18 additions & 22 deletions pkg/controllers/job/plugins/ssh/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,9 @@ func (sp *sshPlugin) OnJobAdd(job *batch.Job) error {
return err
}

if err := helpers.CreateConfigMapIfNotExist(job, sp.Clientset.KubeClients, data, sp.cmName(job)); err != nil {
return err
if err := helpers.CreateSecret(job, sp.Clientset.KubeClients, data, sp.secretName(job)); err != nil {
return fmt.Errorf("create secret for job <%s/%s> with ssh plugin failed for %v",
job.Namespace, job.Name, err)
}

job.Status.ControlledResources["plugin-"+sp.Name()] = sp.Name()
Expand All @@ -94,24 +95,19 @@ func (sp *sshPlugin) OnJobAdd(job *batch.Job) error {
}

func (sp *sshPlugin) OnJobDelete(job *batch.Job) error {
if err := helpers.DeleteConfigmap(job, sp.Clientset.KubeClients, sp.cmName(job)); err != nil {
return err
}

return nil
return helpers.DeleteSecret(job, sp.Clientset.KubeClients, sp.secretName(job))
}

func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
secretName := sp.secretName(job)

cmName := sp.cmName(job)
sshVolume := v1.Volume{
Name: cmName,
Name: secretName,
}

var mode int32 = 0600
sshVolume.ConfigMap = &v1.ConfigMapVolumeSource{
LocalObjectReference: v1.LocalObjectReference{
Name: cmName,
},
sshVolume.Secret = &v1.SecretVolumeSource{
SecretName: secretName,
Items: []v1.KeyToPath{
{
Key: SSHPrivateKey,
Expand All @@ -135,7 +131,7 @@ func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {

if sp.sshKeyFilePath != SSHAbsolutePath {
var noRootMode int32 = 0755
sshVolume.ConfigMap.DefaultMode = &noRootMode
sshVolume.Secret.DefaultMode = &noRootMode
}

pod.Spec.Volumes = append(pod.Spec.Volumes, sshVolume)
Expand All @@ -144,7 +140,7 @@ func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
vm := v1.VolumeMount{
MountPath: sp.sshKeyFilePath,
SubPath: SSHRelativePath,
Name: cmName,
Name: secretName,
}

pod.Spec.Containers[i].VolumeMounts = append(c.VolumeMounts, vm)
Expand All @@ -153,7 +149,7 @@ func (sp *sshPlugin) mountRsaKey(pod *v1.Pod, job *batch.Job) {
return
}

func generateRsaKey(job *batch.Job) (map[string]string, error) {
func generateRsaKey(job *batch.Job) (map[string][]byte, error) {
bitSize := 1024

privateKey, err := rsa.GenerateKey(rand.Reader, bitSize)
Expand All @@ -177,16 +173,16 @@ func generateRsaKey(job *batch.Job) (map[string]string, error) {
}
publicKeyBytes := ssh.MarshalAuthorizedKey(publicRsaKey)

data := make(map[string]string)
data[SSHPrivateKey] = string(privateKeyBytes)
data[SSHPublicKey] = string(publicKeyBytes)
data[SSHConfig] = generateSSHConfig(job)
data := make(map[string][]byte)
data[SSHPrivateKey] = privateKeyBytes
data[SSHPublicKey] = publicKeyBytes
data[SSHConfig] = []byte(generateSSHConfig(job))

return data, nil
}

func (sp *sshPlugin) cmName(job *batch.Job) string {
return fmt.Sprintf("%s-%s", job.Name, sp.Name())
func (sp *sshPlugin) secretName(job *batch.Job) string {
return fmt.Sprintf("%s-%s-%s", job.Name, job.UID, sp.Name())
}

func (sp *sshPlugin) addFlags() {
Expand Down
8 changes: 4 additions & 4 deletions test/e2e/job_plugins.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ var _ = Describe("Job E2E Test: Test Job Plugins", func() {
err := waitJobReady(context, job)
Expect(err).NotTo(HaveOccurred())

pluginName := fmt.Sprintf("%s-ssh", jobName)
_, err = context.kubeclient.CoreV1().ConfigMaps(namespace).Get(
pluginName := fmt.Sprintf("%s-%s-ssh", jobName, job.UID)
_, err = context.kubeclient.CoreV1().Secrets(namespace).Get(
pluginName, v1.GetOptions{})
Expect(err).NotTo(HaveOccurred())

Expand Down Expand Up @@ -206,8 +206,8 @@ var _ = Describe("Job E2E Test: Test Job Plugins", func() {
err := waitJobReady(context, job)
Expect(err).NotTo(HaveOccurred())

pluginName := fmt.Sprintf("%s-ssh", jobName)
_, err = context.kubeclient.CoreV1().ConfigMaps(namespace).Get(
pluginName := fmt.Sprintf("%s-%s-ssh", jobName, job.UID)
_, err = context.kubeclient.CoreV1().Secrets(namespace).Get(
pluginName, v1.GetOptions{})
Expect(err).NotTo(HaveOccurred())

Expand Down

0 comments on commit 519f2a9

Please sign in to comment.