Add pod metadata to replication controller spec template (#193)

* Add pod metadata to replication controller spec template
* Mark affected attributes as deprecated to allow for a migration path

Author: pdecat

CHANGELOG.md

@@ -3,6 +3,7 @@
 IMPROVEMENTS:
 
 * `resource/kubernetes_deployment`, `resource/kubernetes_pod`, `resource/kubernetes_replication_controller`, `resource/kubernetes_stateful_set`: Add `allow_privilege_escalation` to container security contexts attributes ([#249](https://github.com/terraform-providers/terraform-provider-kubernetes/issues/249))
+* Add pod metadata to replication controller spec template ([#193](https://github.com/terraform-providers/terraform-provider-kubernetes/issues/193))
 
 BUG FIXES:
 

kubernetes/resource_kubernetes_deployment.go

@@ -172,7 +172,7 @@ func resourceKubernetesDeployment() *schema.Resource {
 										Required:    true,
 										MaxItems:    1,
 										Elem: &schema.Resource{
-											Schema: podSpecFields(true),
+											Schema: podSpecFields(true, false, false),
 										},
 									},
 								},

kubernetes/resource_kubernetes_pod.go

@@ -38,7 +38,7 @@ func resourceKubernetesPod() *schema.Resource {
 				Required:    true,
 				MaxItems:    1,
 				Elem: &schema.Resource{
-					Schema: podSpecFields(false),
+					Schema: podSpecFields(false, false, false),
 				},
 			},
 		},

kubernetes/resource_kubernetes_replication_controller.go

@@ -63,7 +63,7 @@ func resourceKubernetesReplicationController() *schema.Resource {
 							Required:    true,
 							MaxItems:    1,
 							Elem: &schema.Resource{
-								Schema: podSpecFields(true),
+								Schema: replicationControllerTemplateFieldSpec(),
 							},
 						},
 					},
@@ -73,11 +73,48 @@ func resourceKubernetesReplicationController() *schema.Resource {
 	}
 }
 
+func replicationControllerTemplateFieldSpec() map[string]*schema.Schema {
+	metadata := namespacedMetadataSchema("replication controller's template", true)
+	// TODO: make this required once the legacy fields are removed
+	metadata.Computed = true
+	metadata.Required = false
+	metadata.Optional = true
+
+	templateFields := map[string]*schema.Schema{
+		"metadata": metadata,
+		"spec": {
+			Type:        schema.TypeList,
+			Description: "Spec of the pods managed by the replication controller",
+			Optional:    true, // TODO: make this required once the legacy fields are removed
+			Computed:    true,
+			MaxItems:    1,
+			Elem: &schema.Resource{
+				Schema: podSpecFields(false, false, true),
+			},
+		},
+	}
+
+	// Merge deprecated fields and mark them conflicting with the ones to avoid complex mixed use-cases
+	for k, v := range podSpecFields(true, true, true) {
+		v.ConflictsWith = []string{"spec.0.template.0.spec", "spec.0.template.0.metadata"}
+		templateFields[k] = v
+	}
+
+	return templateFields
+}
+
+func useDeprecatedSpecFields(d *schema.ResourceData) (deprecatedSpecFieldsExist bool) {
+	// Check which replication controller template spec fields are used
+	_, deprecatedSpecFieldsExist = d.GetOkExists("spec.0.template.0.container.0.name")
+	return
+}
+
 func resourceKubernetesReplicationControllerCreate(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*kubernetes.Clientset)
 
 	metadata := expandMetadata(d.Get("metadata").([]interface{}))
-	spec, err := expandReplicationControllerSpec(d.Get("spec").([]interface{}))
+
+	spec, err := expandReplicationControllerSpec(d.Get("spec").([]interface{}), useDeprecatedSpecFields(d))
 	if err != nil {
 		return err
 	}
@@ -135,7 +172,7 @@ func resourceKubernetesReplicationControllerRead(d *schema.ResourceData, meta in
 		return err
 	}
 
-	spec, err := flattenReplicationControllerSpec(rc.Spec)
+	spec, err := flattenReplicationControllerSpec(rc.Spec, useDeprecatedSpecFields(d))
 	if err != nil {
 		return err
 	}
@@ -159,7 +196,7 @@ func resourceKubernetesReplicationControllerUpdate(d *schema.ResourceData, meta
 	ops := patchMetadata("metadata.0.", "/metadata/", d)
 
 	if d.HasChange("spec") {
-		spec, err := expandReplicationControllerSpec(d.Get("spec").([]interface{}))
+		spec, err := expandReplicationControllerSpec(d.Get("spec").([]interface{}), useDeprecatedSpecFields(d))
 		if err != nil {
 			return err
 		}

kubernetes/resource_kubernetes_replication_controller_deprecated_test.go

@@ -4,66 +4,86 @@ import (
 	"github.com/hashicorp/terraform/helper/schema"
 )
 
-func podSpecFields(isUpdatable bool) map[string]*schema.Schema {
+func podSpecFields(isUpdatable, isDeprecated, isComputed bool) map[string]*schema.Schema {
+	var deprecatedMessage string
+	if isDeprecated {
+		deprecatedMessage = "This field is deprecated because template was incorrectly defined as a PodSpec preventing the definition of metadata. Please use the one under the spec field"
+	}
 	s := map[string]*schema.Schema{
 		"active_deadline_seconds": {
 			Type:         schema.TypeInt,
 			Optional:     true,
+			Computed:     isComputed,
 			ValidateFunc: validatePositiveInteger,
 			Description:  "Optional duration in seconds the pod may be active on the node relative to StartTime before the system will actively try to mark it failed and kill associated containers. Value must be a positive integer.",
+			Deprecated:   deprecatedMessage,
 		},
 		"container": {
 			Type:        schema.TypeList,
 			Optional:    true,
+			Computed:    isComputed,
 			Description: "List of containers belonging to the pod. Containers cannot currently be added or removed. There must be at least one container in a Pod. Cannot be updated. More info: http://kubernetes.io/docs/user-guide/containers",
+			Deprecated:  deprecatedMessage,
 			Elem: &schema.Resource{
 				Schema: containerFields(isUpdatable, false),
 			},
 		},
 		"init_container": {
 			Type:        schema.TypeList,
 			Optional:    true,
+			Computed:    isComputed,
 			ForceNew:    true,
 			Description: "List of init containers belonging to the pod. Init containers always run to completion and each must complete successfully before the next is started. More info: https://kubernetes.io/docs/concepts/workloads/pods/init-containers/",
+			Deprecated:  deprecatedMessage,
 			Elem: &schema.Resource{
 				Schema: containerFields(isUpdatable, true),
 			},
 		},
 		"dns_policy": {
 			Type:        schema.TypeString,
 			Optional:    true,
-			Default:     "ClusterFirst",
+			Computed:    isComputed,
+			DefaultFunc: defaultIfNotComputed(isComputed, "ClusterFirst"),
 			Description: "Set DNS policy for containers within the pod. One of 'ClusterFirst' or 'Default'. Defaults to 'ClusterFirst'.",
+			Deprecated:  deprecatedMessage,
 		},
 		"host_ipc": {
 			Type:        schema.TypeBool,
 			Optional:    true,
-			Default:     false,
+			Computed:    isComputed,
+			DefaultFunc: defaultIfNotComputed(isComputed, false),
 			Description: "Use the host's ipc namespace. Optional: Default to false.",
+			Deprecated:  deprecatedMessage,
 		},
 		"host_network": {
 			Type:        schema.TypeBool,
 			Optional:    true,
-			Default:     false,
+			Computed:    isComputed,
+			DefaultFunc: defaultIfNotComputed(isComputed, false),
 			Description: "Host networking requested for this pod. Use the host's network namespace. If this option is set, the ports that will be used must be specified.",
+			Deprecated:  deprecatedMessage,
 		},
 
 		"host_pid": {
 			Type:        schema.TypeBool,
 			Optional:    true,
-			Default:     false,
+			Computed:    isComputed,
+			DefaultFunc: defaultIfNotComputed(isComputed, false),
 			Description: "Use the host's pid namespace.",
+			Deprecated:  deprecatedMessage,
 		},
 
 		"hostname": {
 			Type:        schema.TypeString,
 			Optional:    true,
 			Computed:    true,
 			Description: "Specifies the hostname of the Pod If not specified, the pod's hostname will be set to a system-defined value.",
+			Deprecated:  deprecatedMessage,
 		},
 		"image_pull_secrets": {
 			Type:        schema.TypeList,
 			Description: "ImagePullSecrets is an optional list of references to secrets in the same namespace to use for pulling any of the images used by this PodSpec. If specified, these secrets will be passed to individual puller implementations for them to use. For example, in the case of docker, only DockerConfig type secrets are honored. More info: http://kubernetes.io/docs/user-guide/images#specifying-imagepullsecrets-on-a-pod",
+			Deprecated:  deprecatedMessage,
 			Optional:    true,
 			Computed:    true,
 			Elem: &schema.Resource{
@@ -81,23 +101,30 @@ func podSpecFields(isUpdatable bool) map[string]*schema.Schema {
 			Optional:    true,
 			Computed:    true,
 			Description: "NodeName is a request to schedule this pod onto a specific node. If it is non-empty, the scheduler simply schedules this pod onto that node, assuming that it fits resource requirements.",
+			Deprecated:  deprecatedMessage,
 		},
 		"node_selector": {
 			Type:        schema.TypeMap,
 			Optional:    true,
+			Computed:    isComputed,
 			Description: "NodeSelector is a selector which must be true for the pod to fit on a node. Selector which must match a node's labels for the pod to be scheduled on that node. More info: http://kubernetes.io/docs/user-guide/node-selection.",
+			Deprecated:  deprecatedMessage,
 		},
 		"restart_policy": {
 			Type:        schema.TypeString,
 			Optional:    true,
-			Default:     "Always",
+			Computed:    isComputed,
+			DefaultFunc: defaultIfNotComputed(isComputed, "Always"),
 			Description: "Restart policy for all containers within the pod. One of Always, OnFailure, Never. More info: http://kubernetes.io/docs/user-guide/pod-states#restartpolicy.",
+			Deprecated:  deprecatedMessage,
 		},
 		"security_context": {
 			Type:        schema.TypeList,
 			Optional:    true,
+			Computed:    isComputed,
 			MaxItems:    1,
 			Description: "SecurityContext holds pod-level security attributes and common container settings. Optional: Defaults to empty",
+			Deprecated:  deprecatedMessage,
 			Elem: &schema.Resource{
 				Schema: map[string]*schema.Schema{
 					"fs_group": {
@@ -140,24 +167,31 @@ func podSpecFields(isUpdatable bool) map[string]*schema.Schema {
 			Optional:    true,
 			Computed:    true,
 			Description: "ServiceAccountName is the name of the ServiceAccount to use to run this pod. More info: http://releases.k8s.io/HEAD/docs/design/service_accounts.md.",
+			Deprecated:  deprecatedMessage,
 		},
 		"subdomain": {
 			Type:        schema.TypeString,
 			Optional:    true,
+			Computed:    isComputed,
 			Description: `If specified, the fully qualified Pod hostname will be "...svc.". If not specified, the pod will not have a domainname at all..`,
+			Deprecated:  deprecatedMessage,
 		},
 		"termination_grace_period_seconds": {
 			Type:         schema.TypeInt,
 			Optional:     true,
-			Default:      30,
+			Computed:     isComputed,
+			DefaultFunc:  defaultIfNotComputed(isComputed, 30),
 			ValidateFunc: validateTerminationGracePeriodSeconds,
 			Description:  "Optional duration in seconds the pod needs to terminate gracefully. May be decreased in delete request. Value must be non-negative integer. The value zero indicates delete immediately. If this value is nil, the default grace period will be used instead. The grace period is the duration in seconds after the processes running in the pod are sent a termination signal and the time when the processes are forcibly halted with a kill signal. Set this value longer than the expected cleanup time for your process.",
+			Deprecated:   deprecatedMessage,
 		},
 
 		"volume": {
 			Type:        schema.TypeList,
 			Optional:    true,
+			Computed:    isComputed,
 			Description: "List of volumes that can be mounted by containers belonging to the pod. More info: http://kubernetes.io/docs/user-guide/volumes",
+			Deprecated:  deprecatedMessage,
 			Elem:        volumeSchema(),
 		},
 	}
@@ -179,6 +213,16 @@ func podSpecFields(isUpdatable bool) map[string]*schema.Schema {
 	return s
 }
 
+func defaultIfNotComputed(isComputed bool, defaultValue interface{}) schema.SchemaDefaultFunc {
+	return func() (interface{}, error) {
+		if isComputed {
+			return nil, nil
+		}
+
+		return defaultValue, nil
+	}
+}
+
 func volumeSchema() *schema.Resource {
 	v := commonVolumeSources()
 

kubernetes/resource_kubernetes_replication_controller_test.go

@@ -13,7 +13,7 @@ func podTemplateFields(isUpdatable bool) map[string]*schema.Schema {
 			Optional:    true,
 			MaxItems:    1,
 			Elem: &schema.Resource{
-				Schema: podSpecFields(false),
+				Schema: podSpecFields(false, false, false),
 			},
 		},
 	}

kubernetes/schema_pod_spec.go

@@ -1,29 +1,48 @@
 package kubernetes
 
 import (
+	"errors"
+
 	"k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func flattenReplicationControllerSpec(in v1.ReplicationControllerSpec) ([]interface{}, error) {
+func flattenReplicationControllerSpec(in v1.ReplicationControllerSpec, useDeprecatedSpecFields bool) ([]interface{}, error) {
 	att := make(map[string]interface{})
 	att["min_ready_seconds"] = in.MinReadySeconds
 
 	if in.Replicas != nil {
 		att["replicas"] = *in.Replicas
 	}
 
-	att["selector"] = in.Selector
-	podSpec, err := flattenPodSpec(in.Template.Spec)
-	if err != nil {
-		return nil, err
+	if in.Selector != nil {
+		att["selector"] = in.Selector
+	}
+
+	if in.Template != nil {
+		podSpec, err := flattenPodSpec(in.Template.Spec)
+		if err != nil {
+			return nil, err
+		}
+		template := make(map[string]interface{})
+
+		if useDeprecatedSpecFields {
+			// Use deprecated fields
+			for k, v := range podSpec[0].(map[string]interface{}) {
+				template[k] = v
+			}
+		} else {
+			// Use new fields
+			template["spec"] = podSpec
+			template["metadata"] = flattenMetadata(in.Template.ObjectMeta)
+		}
+
+		att["template"] = []interface{}{template}
 	}
-	att["template"] = podSpec
 
 	return []interface{}{att}, nil
 }
 
-func expandReplicationControllerSpec(rc []interface{}) (*v1.ReplicationControllerSpec, error) {
+func expandReplicationControllerSpec(rc []interface{}, useDeprecatedSpecFields bool) (*v1.ReplicationControllerSpec, error) {
 	obj := &v1.ReplicationControllerSpec{}
 	if len(rc) == 0 || rc[0] == nil {
 		return obj, nil
@@ -32,15 +51,49 @@ func expandReplicationControllerSpec(rc []interface{}) (*v1.ReplicationControlle
 	obj.MinReadySeconds = int32(in["min_ready_seconds"].(int))
 	obj.Replicas = ptrToInt32(int32(in["replicas"].(int)))
 	obj.Selector = expandStringMap(in["selector"].(map[string]interface{}))
-	podSpec, err := expandPodSpec(in["template"].([]interface{}))
+
+	template, err := expandReplicationControllerTemplate(in["template"].([]interface{}), obj.Selector, useDeprecatedSpecFields)
 	if err != nil {
 		return obj, err
 	}
-	obj.Template = &v1.PodTemplateSpec{
-		ObjectMeta: metav1.ObjectMeta{
-			Labels: obj.Selector,
-		},
-		Spec: *podSpec,
+
+	obj.Template = template
+
+	return obj, nil
+}
+
+func expandReplicationControllerTemplate(rct []interface{}, selector map[string]string, useDeprecatedSpecFields bool) (*v1.PodTemplateSpec, error) {
+	obj := &v1.PodTemplateSpec{}
+
+	if useDeprecatedSpecFields {
+		// Add labels from selector to ensure proper selection of pods by the replication controller for deprecated use case
+		obj.ObjectMeta.Labels = selector
+
+		// Get pod spec from deprecated fields
+		podSpecDeprecated, err := expandPodSpec(rct)
+		if err != nil {
+			return obj, err
+		}
+		obj.Spec = *podSpecDeprecated
+	} else {
+		in := rct[0].(map[string]interface{})
+		metadata := in["metadata"].([]interface{})
+
+		// Return an error if new spec fields are used but no metadata is defined to preserve the Required property of the metadata field
+		// cf. https://www.terraform.io/docs/extend/best-practices/deprecations.html#renaming-a-required-attribute
+		if len(metadata) < 1 {
+			return obj, errors.New("`spec.template.metadata` is Required when new 'spec.template.spec' fields are used.")
+		}
+
+		// Get user defined metadata
+		obj.ObjectMeta = expandMetadata(metadata)
+
+		// Get pod spec from new fields
+		podSpec, err := expandPodSpec(in["spec"].([]interface{}))
+		if err != nil {
+			return obj, err
+		}
+		obj.Spec = *podSpec
 	}
 
 	return obj, nil