Request Changes - File Assets

- removed the Mode field from the FileAsset spec - removed the ability to template the content - removed the need to specify the Path and instead default to /srv/kubernetes/assets/<name> - change the FileAssets from []*FileAssets to []FileAssets
kubernetes · Aug 18, 2017 · 17e0524 · 17e0524
1 parent 41c3262
commit 17e0524
Show file tree

Hide file tree

Showing 12 changed files with 50 additions and 275 deletions.
diff --git a/docs/cluster_spec.md b/docs/cluster_spec.md
@@ -251,14 +251,14 @@ spec:
 
 ### fileAssets
 
-FileAssets permit you to place inline file content into the cluster and instanceGroup specification, which can be consumed on the nodes
+FileAssets is an alpha feature which permits you to place inline file content into the cluster and instanceGroup specification. It's desiginated as alpha as you can probably do this via kubernetes daemonsets as an alternative.
 
 ```yaml
 spec:
   fileAssets:
   - name: iptable-restore
+    # Note if not path is specificied the default path it /srv/kubernetes/assets/<name>
     path: /var/lib/iptables/rules-save
-    mode: 0440
     roles: [Master,Node,Bastion] # a list of roles to apply the asset to, zero defaults to all
     content: |
       some file content
@@ -267,7 +267,6 @@ spec:
   - name: iptable-restore
     path: /var/lib/iptables/rules-save
     templated: true
-    mode: 0440
     content: |
       some file content
       *filter

diff --git a/nodeup/pkg/model/file_assets.go b/nodeup/pkg/model/file_assets.go
@@ -18,18 +18,21 @@ package model
 
 import (
 	"encoding/base64"
-	"errors"
 	"fmt"
 	"path/filepath"
 	"strings"
 	"text/template"
 
 	"k8s.io/kops/pkg/apis/kops"
-	"k8s.io/kops/pkg/model"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/nodeup/nodetasks"
 )
 
+const (
+	// FileAssetsDefaultPath is the default location for assets which have no path
+	FileAssetsDefaultPath string = "/srv/kubernetes/assets"
+)
+
 // FileAssetsBuilder configures the hooks
 type FileAssetsBuilder struct {
 	*NodeupModelContext
@@ -41,6 +44,12 @@ var _ fi.ModelBuilder = &FileAssetsBuilder{}
 func (f *FileAssetsBuilder) Build(c *fi.ModelBuilderContext) error {
 	// used to keep track of previous file, so a instanceGroup can override a cluster wide one
 	tracker := make(map[string]bool, 0)
+	// ensure the default path exists
+	c.AddTask(&nodetasks.File{
+		Path: filepath.Dir(FileAssetsDefaultPath),
+		Type: nodetasks.FileType_Directory,
+		Mode: s("0755"),
+	})
 	// do we have any instanceGroup file assets
 	if f.InstanceGroup.Spec.FileAssets != nil {
 		if err := f.buildFileAssets(c, f.InstanceGroup.Spec.FileAssets, tracker); err != nil {
@@ -57,36 +66,23 @@ func (f *FileAssetsBuilder) Build(c *fi.ModelBuilderContext) error {
 }
 
 // buildFileAssets is responsible for rendering the file assets to disk
-func (f *FileAssetsBuilder) buildFileAssets(c *fi.ModelBuilderContext, assets []*kops.FileAssetSpec, tracker map[string]bool) error {
+func (f *FileAssetsBuilder) buildFileAssets(c *fi.ModelBuilderContext, assets []kops.FileAssetSpec, tracker map[string]bool) error {
 	for _, asset := range assets {
-		if err := validateFileAsset(asset); err != nil {
-			return fmt.Errorf("The file asset is invalid, name: %s, error: %q", asset.Name, err)
-		}
 		// @check if the file asset applys to us. If no roles applied we assume its applied to all roles
-		// @todo: use the containsRole when the hooks PR is merged
-		if len(asset.Roles) > 0 {
-			var found bool
-			for _, x := range asset.Roles {
-				if f.InstanceGroup.Spec.Role == x {
-					found = true
-					break
-				}
-			}
-			if !found {
-				continue
-			}
+		if len(asset.Roles) > 0 && !containsRole(f.InstanceGroup.Spec.Role, asset.Roles) {
+			continue
+		}
+		// @check if e have a path and if not use the default path
+		assetPath := asset.Path
+		if assetPath == "" {
+			assetPath = fmt.Sprintf("%s/%s", FileAssetsDefaultPath, asset.Name)
 		}
-
 		// @check if the file has already been done and skip
-		if _, found := tracker[asset.Path]; found {
+		if _, found := tracker[assetPath]; found {
 			continue
 		}
-		tracker[asset.Path] = true // update the tracker
+		tracker[assetPath] = true // update the tracker
 
-		// fill in the defaults for the file perms
-		if asset.Mode == "" {
-			asset.Mode = "0400"
-		}
 		// @check is the contents requires decoding
 		content := asset.Content
 		if asset.IsBase64 {
@@ -99,33 +95,15 @@ func (f *FileAssetsBuilder) buildFileAssets(c *fi.ModelBuilderContext, assets []
 
 		// @check if the directory structure exist or create it
 		c.AddTask(&nodetasks.File{
-			Path: filepath.Dir(asset.Path),
+			Path: filepath.Dir(assetPath),
 			Type: nodetasks.FileType_Directory,
 			Mode: s("0755"),
 		})
 
-		var resource fi.Resource
-		var err error
-		switch asset.Templated {
-		case true:
-			resource, err = f.getRenderedResource(content)
-			if err != nil {
-				return fmt.Errorf("Failed on file assets: %s, build rendered resource, error: %q", asset.Name, err)
-			}
-		default:
-			resource = fi.NewStringResource(content)
-		}
-
-		// @check the file permissions
-		perms := asset.Mode
-		if !strings.HasPrefix(perms, "0") {
-			perms = fmt.Sprintf("%d%s", 0, perms)
-		}
-
 		c.AddTask(&nodetasks.File{
-			Contents: resource,
-			Mode:     s(perms),
-			Path:     asset.Path,
+			Contents: fi.NewStringResource(content),
+			Mode:     s("0440"),
+			Path:     assetPath,
 			Type:     nodetasks.FileType_File,
 		})
 	}
@@ -138,32 +116,3 @@ var templateFuncs = template.FuncMap{
 	"split": strings.Split,
 	"join":  strings.Join,
 }
-
-// getRenderedResource is responsible for rendering the content if templated
-func (f *FileAssetsBuilder) getRenderedResource(content string) (fi.Resource, error) {
-	context := map[string]interface{}{
-		"Cluster":       f.Cluster.Spec,
-		"InstanceGroup": f.InstanceGroup.Spec,
-		"Master":        fmt.Sprintf("%t", f.IsMaster),
-		"Name":          f.InstanceGroup.Name,
-	}
-
-	resource, err := model.NewTemplateResource("FileAsset", content, templateFuncs, context)
-	if err != nil {
-		return nil, err
-	}
-
-	return resource, nil
-}
-
-// validateFileAsset performs some basic validation on the asset
-func validateFileAsset(asset *kops.FileAssetSpec) error {
-	if asset.Path == "" {
-		return errors.New("does not have a path")
-	}
-	if asset.Content == "" {
-		return errors.New("does not have any contents")
-	}
-
-	return nil
-}
diff --git a/nodeup/pkg/model/file_assets_test.go b/nodeup/pkg/model/file_assets_test.go
diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go
@@ -76,7 +76,6 @@ type ClusterSpec struct {
 	KeyStore string `json:"keyStore,omitempty"`
 	// ConfigStore is the VFS path to where the configuration (Cluster, InstanceGroups etc) is stored
 	ConfigStore string `json:"configStore,omitempty"`
-
 	// DNSZone is the DNS zone we should use when configuring DNS
 	// This is because some clouds let us define a managed zone foo.bar, and then have
 	// kubernetes.dev.foo.bar, without needing to define dev.foo.bar as a hosted zone.
@@ -114,7 +113,7 @@ type ClusterSpec struct {
 	// Additional policies to add for roles
 	AdditionalPolicies *map[string]string `json:"additionalPolicies,omitempty"`
 	// A collection of files assets for deployed cluster wide
-	FileAssets []*FileAssetSpec `json:"fileAssets,omitempty"`
+	FileAssets []FileAssetSpec `json:"fileAssets,omitempty"`
 	// EtcdClusters stores the configuration for each cluster
 	EtcdClusters []*EtcdClusterSpec `json:"etcdClusters,omitempty"`
 	// Component configurations
@@ -130,7 +129,6 @@ type ClusterSpec struct {
 
 	// Networking configuration
 	Networking *NetworkingSpec `json:"networking,omitempty"`
-
 	// API field controls how the API is exposed outside the cluster
 	API *AccessSpec `json:"api,omitempty"`
 	// Authentication field controls how the cluster is configured for authentication
@@ -153,14 +151,10 @@ type FileAssetSpec struct {
 	Name string `json:"name,omitempty"`
 	// Path is the location this file should reside
 	Path string `json:"path,omitempty"`
-	// Mode is unix filemode for the file - defaults to 0400
-	Mode string `json:"mode,omitempty"`
 	// Roles is a list of roles the file asset should be applied, defaults to all
 	Roles []InstanceGroupRole `json:"roles,omitempty"`
 	// Content is the contents of the file
 	Content string `json:"content,omitempty"`
-	// Templated indicates the content is templated
-	Templated bool `json:"templated,omitempty"`
 	// IsBase64 indicates the contents is base64 encoded
 	IsBase64 bool `json:"isBase64,omitempty"`
 }

diff --git a/pkg/apis/kops/instancegroup.go b/pkg/apis/kops/instancegroup.go
@@ -95,7 +95,7 @@ type InstanceGroupSpec struct {
 	// NodeLabels indicates the kubernetes labels for nodes in this group
 	NodeLabels map[string]string `json:"nodeLabels,omitempty"`
 	// FileAssets is a collection of file assets for this instance group
-	FileAssets []*FileAssetSpec `json:"fileAssets,omitempty"`
+	FileAssets []FileAssetSpec `json:"fileAssets,omitempty"`
 	// Describes the tenancy of the instance group. Can be either default or dedicated.
 	// Currently only applies to AWS.
 	Tenancy string `json:"tenancy,omitempty"`

diff --git a/pkg/apis/kops/v1alpha1/cluster.go b/pkg/apis/kops/v1alpha1/cluster.go
@@ -135,7 +135,7 @@ type ClusterSpec struct {
 	AdditionalPolicies *map[string]string `json:"additionalPolicies,omitempty"`
 
 	// A collection of files assets for deployed cluster wide
-	FileAssets []*FileAssetSpec `json:"fileAssets,omitempty"`
+	FileAssets []FileAssetSpec `json:"fileAssets,omitempty"`
 
 	//HairpinMode                   string `json:",omitempty"`
 	//
@@ -260,14 +260,10 @@ type FileAssetSpec struct {
 	Name string `json:"name,omitempty"`
 	// Path is the location this file should reside
 	Path string `json:"path,omitempty"`
-	// Mode is unix filemode for the file - defaults to 0400
-	Mode string `json:"mode,omitempty"`
 	// Roles is a list of roles the file asset should be applied, defaults to all
 	Roles []InstanceGroupRole `json:"roles,omitempty"`
 	// Content is the contents of the file
 	Content string `json:"content,omitempty"`
-	// Templated indicates the content is templated
-	Templated bool `json:"templated,omitempty"`
 	// IsBase64 indicates the contents is base64 encoded
 	IsBase64 bool `json:"isBase64,omitempty"`
 }

diff --git a/pkg/apis/kops/v1alpha1/instancegroup.go b/pkg/apis/kops/v1alpha1/instancegroup.go
@@ -78,7 +78,7 @@ type InstanceGroupSpec struct {
 	// NodeLabels indicates the kubernetes labels for nodes in this group
 	NodeLabels map[string]string `json:"nodeLabels,omitempty"`
 	// A collection of files assets for deployed cluster wide
-	FileAssets []*FileAssetSpec `json:"fileAssets,omitempty"`
+	FileAssets []FileAssetSpec `json:"fileAssets,omitempty"`
 	// Describes the tenancy of the instance group. Can be either default or dedicated.
 	// Currently only applies to AWS.
 	Tenancy string `json:"tenancy,omitempty"`