Refactor expander

cmd/manager/manager/expander.go

@@ -17,65 +17,53 @@ limitations under the License.
 package manager
 
 import (
+	"github.com/kubernetes/helm/pkg/common"
+	"github.com/kubernetes/helm/pkg/expansion"
+	"github.com/kubernetes/helm/pkg/repo"
+
 	"bytes"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net/http"
-
-	"github.com/ghodss/yaml"
-	"github.com/kubernetes/helm/pkg/common"
 )
 
+/*
 const (
 	// TODO (iantw): Align this with a character not allowed to show up in resource names.
 	layoutNodeKeySeparator = "#"
 )
+*/
 
-// ExpandedTemplate is the structure returned by the expansion service.
-type ExpandedTemplate struct {
+// ExpandedConfiguration is the structure returned by the expansion service.
+type ExpandedConfiguration struct {
 	Config *common.Configuration `json:"config"`
 	Layout *common.Layout        `json:"layout"`
 }
 
 // Expander abstracts interactions with the expander and deployer services.
 type Expander interface {
-	ExpandTemplate(t *common.Template) (*ExpandedTemplate, error)
-}
-
-// TODO: Remove mockResolver when type resolver is completely excised
-type mockResolver struct {
-}
-
-func (r *mockResolver) ResolveTypes(c *common.Configuration, i []*common.ImportFile) ([]*common.ImportFile, error) {
-	return nil, nil
+	ExpandConfiguration(conf *common.Configuration) (*ExpandedConfiguration, error)
 }
 
 // NewExpander returns a new initialized Expander.
-func NewExpander(url string) Expander {
-	tr := &mockResolver{}
-	return &expander{url, tr}
+func NewExpander(URL string, rp repo.IRepoProvider) Expander {
+	if rp == nil {
+		rp = repo.NewRepoProvider(nil, nil, nil)
+	}
+
+	return &expander{expanderURL: URL, repoProvider: rp}
 }
 
 type expander struct {
+	repoProvider repo.IRepoProvider
 	expanderURL  string
-	typeResolver TypeResolver
-}
-
-// TypeResolver finds Types in a Configuration which aren't yet reduceable to an import file
-// or primitive, and attempts to replace them with a template from a URL.
-type TypeResolver interface {
-	ResolveTypes(config *common.Configuration, imports []*common.ImportFile) ([]*common.ImportFile, error)
 }
 
 func (e *expander) getBaseURL() string {
 	return fmt.Sprintf("%s/expand", e.expanderURL)
 }
 
-func expanderError(t *common.Template, err error) error {
-	return fmt.Errorf("cannot expand template named %s (%s):\n%s", t.Name, err, t.Content)
-}
-
 // ExpanderResponse gives back a layout, which has nested structure
 // Resource0
 //		ResourceDefinition
@@ -108,6 +96,7 @@ func expanderError(t *common.Template, err error) error {
 // between the name#template key to exist in the layout given a particular choice of naming.
 // In practice, it would be nearly impossible to hit, but consider including properties/name/type
 // into a hash of sorts to make this robust...
+/*
 func walkLayout(l *common.Layout, imports []*common.ImportFile, toReplace map[string]*common.LayoutResource) map[string]*common.LayoutResource {
 	ret := map[string]*common.LayoutResource{}
 	toVisit := l.Resources
@@ -126,81 +115,68 @@ func walkLayout(l *common.Layout, imports []*common.ImportFile, toReplace map[st
 
 	return ret
 }
+*/
 
-// isTemplate returns whether a given type is a template.
-func isTemplate(t string, imports []*common.ImportFile) bool {
-	for _, imp := range imports {
-		if imp.Name == t {
-			return true
-		}
+// ExpandConfiguration expands the supplied configuration and returns
+// an expanded configuration.
+func (e *expander) ExpandConfiguration(conf *common.Configuration) (*ExpandedConfiguration, error) {
+	expConf, err := e.expandConfiguration(conf)
+	if err != nil {
+		return nil, fmt.Errorf("cannot expand configuration:%s\n%v\n", err, conf)
 	}
 
-	return false
+	return expConf, nil
 }
 
-// ExpandTemplate expands the supplied template, and returns a configuration.
-// It will also update the imports in the provided template if any were added
-// during type resolution.
-func (e *expander) ExpandTemplate(t *common.Template) (*ExpandedTemplate, error) {
-	// We have a fencepost problem here.
-	// 1. Start by trying to resolve any missing templates
-	// 2. Expand the configuration using all the of the imports available to us at this point
-	// 3. Expansion may yield additional templates, so we run the type resolution again
-	// 4. If type resolution resulted in new imports being available, return to 2.
-	config := &common.Configuration{}
-	if err := yaml.Unmarshal([]byte(t.Content), config); err != nil {
-		e := fmt.Errorf("Unable to unmarshal configuration (%s): %s", err, t.Content)
-		return nil, e
-	}
+func (e *expander) expandConfiguration(conf *common.Configuration) (*ExpandedConfiguration, error) {
+	resources := []*common.Resource{}
+	layout := []*common.LayoutResource{}
 
-	var finalLayout *common.Layout
-	needResolve := map[string]*common.LayoutResource{}
+	for _, resource := range conf.Resources {
+		if !repo.IsChartReference(resource.Type) {
+			resources = append(resources, resource)
+			continue
+		}
 
-	// Start things off by attempting to resolve the templates in a first pass.
-	newImp, err := e.typeResolver.ResolveTypes(config, t.Imports)
+		cbr, _, err := e.repoProvider.GetChartByReference(resource.Type)
 		if err != nil {
-		e := fmt.Errorf("type resolution failed: %s", err)
-		return nil, expanderError(t, e)
+			return nil, err
 		}
 
-	t.Imports = append(t.Imports, newImp...)
-
-	for {
-		// Now expand with everything imported.
-		result, err := e.expandTemplate(t)
+		defer cbr.Close()
+		content, err := cbr.LoadContent()
 		if err != nil {
-			e := fmt.Errorf("template expansion: %s", err)
-			return nil, expanderError(t, e)
+			return nil, err
 		}
 
-		// Once we set this layout, we're operating on the "needResolve" *LayoutResources,
-		// which are pointers into the original layout structure. After each expansion we
-		// lose the templates in the previous expansion, so we have to keep the first one
-		// around and keep appending to the pointers in it as we get more layers of expansion.
-		if finalLayout == nil {
-			finalLayout = result.Layout
+		svcReq := &expansion.ServiceRequest{
+			ChartInvocation: resource,
+			Chart:           content,
 		}
-		needResolve = walkLayout(result.Layout, t.Imports, needResolve)
 
-		newImp, err = e.typeResolver.ResolveTypes(result.Config, t.Imports)
+		svcResp, err := e.callService(svcReq)
 		if err != nil {
-			e := fmt.Errorf("type resolution failed: %s", err)
-			return nil, expanderError(t, e)
+			return nil, err
 		}
 
-		// If the new imports contain nothing, we are done. Everything is fully expanded.
-		if len(newImp) == 0 {
-			result.Layout = finalLayout
-			return result, nil
+		expConf, err := e.expandConfiguration(svcResp)
+		if err != nil {
+			return nil, err
 		}
 
-		// Update imports with any new imports from type resolution.
-		t.Imports = append(t.Imports, newImp...)
+		// TODO: build up layout hiearchically
+		resources = append(resources, expConf.Config.Resources...)
+		layout = append(layout, expConf.Layout.Resources...)
 	}
+
+	return &ExpandedConfiguration{
+		Config: &common.Configuration{Resources: resources},
+		Layout: &common.Layout{Resources: layout},
+	}, nil
 }
 
-func (e *expander) expandTemplate(t *common.Template) (*ExpandedTemplate, error) {
-	j, err := json.Marshal(t)
+func (e *expander) callService(svcReq *expansion.ServiceRequest) (*common.Configuration, error) {
+	j, err := json.Marshal(svcReq)
 	if err != nil {
 		return nil, err
 	}
@@ -232,37 +208,11 @@ func (e *expander) expandTemplate(t *common.Template) (*ExpandedTemplate, error)
 		return nil, err
 	}
 
-	er := &ExpansionResponse{}
-	if err := json.Unmarshal(body, er); err != nil {
+	svcResp := &common.Configuration{}
+	if err := json.Unmarshal(body, svcResp); err != nil {
 		e := fmt.Errorf("cannot unmarshal response body (%s):%s", err, body)
 		return nil, e
 	}
 
-	template, err := er.Unmarshal()
-	if err != nil {
-		e := fmt.Errorf("cannot unmarshal response yaml (%s):%v", err, er)
-		return nil, e
-	}
-
-	return template, nil
-}
-
-// ExpansionResponse describes the results of marshaling an ExpandedTemplate.
-type ExpansionResponse struct {
-	Config string `json:"config"`
-	Layout string `json:"layout"`
-}
-
-// Unmarshal creates and returns an ExpandedTemplate from an ExpansionResponse.
-func (er *ExpansionResponse) Unmarshal() (*ExpandedTemplate, error) {
-	template := &ExpandedTemplate{}
-	if err := yaml.Unmarshal([]byte(er.Config), &template.Config); err != nil {
-		return nil, fmt.Errorf("cannot unmarshal config (%s):\n%s", err, er.Config)
-	}
-
-	if err := yaml.Unmarshal([]byte(er.Layout), &template.Layout); err != nil {
-		return nil, fmt.Errorf("cannot unmarshal layout (%s):\n%s", err, er.Layout)
-	}
-
-	return template, nil
+	return svcResp, nil
 }

cmd/manager/manager/expander_test.go

@@ -26,44 +26,24 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/ghodss/yaml"
+	"github.com/kubernetes/helm/pkg/chart"
 	"github.com/kubernetes/helm/pkg/common"
+	"github.com/kubernetes/helm/pkg/expansion"
+	"github.com/kubernetes/helm/pkg/repo"
 	"github.com/kubernetes/helm/pkg/util"
-
-	"github.com/ghodss/yaml"
 )
 
-var validTemplateTestCaseData = common.Template{
-	Name:    "TestTemplate",
-	Content: string(validContentTestCaseData),
-	Imports: validImportFilesTestCaseData,
-}
-
-var validContentTestCaseData = []byte(`
-imports:
-- path: test-type.py
-resources:
-- name: test
-  type: test-type.py
-  properties:
-    test-property: test-value
-`)
-
-var validImportFilesTestCaseData = []*common.ImportFile{
-	{
-		Name:    "test-type.py",
-		Content: "test-type.py validTemplateTestCaseData content",
-	},
-	{
-		Name:    "test.py",
-		Content: "test.py validTemplateTestCaseData content",
-	},
-	{
-		Name:    "test2.py",
-		Content: "test2.py validTemplateTestCaseData content",
-	},
-}
+var (
+	TestRepoBucket   = "kubernetes-charts-testing"
+	TestRepoURL      = "gs://" + TestRepoBucket
+	TestChartName    = "frobnitz"
+	TestChartVersion = "0.0.1"
+	TestArchiveName  = TestChartName + "-" + TestChartVersion + ".tgz"
+	TestResourceType = TestRepoURL + "/" + TestArchiveName
+)
 
-var validConfigTestCaseData = []byte(`
+var validResponseTestCaseData = []byte(`
 resources:
 - name: test-service
   properties:
@@ -91,6 +71,7 @@ resources:
   type: ReplicationController
 `)
 
+/*
 var validLayoutTestCaseData = []byte(`
 resources:
 - name: test
@@ -126,15 +107,9 @@ resources:
   type: test2.jinja
 `)
 
-var validResponseTestCaseData = ExpansionResponse{
-	Config: string(validConfigTestCaseData),
-	Layout: string(validLayoutTestCaseData),
-}
-
 var roundTripContent = `
-config:
-  resources:
-  - name: test
+resources:
+- name: test
   type: test.py
   properties:
     test: test
@@ -207,35 +182,58 @@ layout:
           test: test
 `
 
-var roundTripTemplate = common.Template{
-	Name:    "TestTemplate",
-	Content: roundTripContent,
-	Imports: nil,
+var roundTripResponse = &ExpandedConfiguration{
+	Config: roundTripExpanded,
+}
+
+var roundTripResponse2 = &ExpandedConfiguration{
+	Config: roundTripExpanded2,
+}
+
+var roundTripResponses = []*ExpandedConfiguration{
+	roundTripResponse,
+	roundTripResponse2,
+}
+*/
+
+type mockRepoProvider struct {
+}
+
+func (m *mockRepoProvider) GetChartByReference(reference string) (*chart.Chart, repo.IChartRepo, error) {
+	return &chart.Chart{}, nil, nil
+}
+
+func (m *mockRepoProvider) GetRepoByChartURL(URL string) (repo.IChartRepo, error) {
+	return nil, nil
+}
+
+func (m *mockRepoProvider) GetRepoByURL(URL string) (repo.IChartRepo, error) {
+	return nil, nil
 }
 
 type ExpanderTestCase struct {
 	Description   string
 	Error         string
 	Handler       func(w http.ResponseWriter, r *http.Request)
-	ValidResponse *ExpandedTemplate
+	ValidResponse *ExpandedConfiguration
 }
 
 func TestExpandTemplate(t *testing.T) {
-	roundTripResponse := &ExpandedTemplate{}
-	if err := yaml.Unmarshal([]byte(finalExpanded), roundTripResponse); err != nil {
-		panic(err)
-	}
+	//	roundTripResponse := &ExpandedConfiguration{}
+	//	if err := yaml.Unmarshal([]byte(finalExpanded), roundTripResponse); err != nil {
+	//		panic(err)
+	//	}
 
 	tests := []ExpanderTestCase{
 		{
-			"expect success for ExpandTemplate",
+			"expect success for ExpandConfiguration",
 			"",
 			expanderSuccessHandler,
-			getValidResponse(t, "expect success for ExpandTemplate"),
+			getValidExpandedConfiguration(),
 		},
 		{
-			"expect error for ExpandTemplate",
-			"cannot expand template",
+			"expect error for ExpandConfiguration",
+			"simulated failure",
 			expanderErrorHandler,
 			nil,
 		},
@@ -245,12 +243,23 @@ func TestExpandTemplate(t *testing.T) {
 		ts := httptest.NewServer(http.HandlerFunc(etc.Handler))
 		defer ts.Close()
 
-		expander := NewExpander(ts.URL)
-		actualResponse, err := expander.ExpandTemplate(&validTemplateTestCaseData)
+		expander := NewExpander(ts.URL, nil)
+		resource := &common.Resource{
+			Name: "test_invocation",
+			Type: TestResourceType,
+		}
+
+		conf := &common.Configuration{
+			Resources: []*common.Resource{
+				resource,
+			},
+		}
+
+		actualResponse, err := expander.ExpandConfiguration(conf)
 		if err != nil {
 			message := err.Error()
 			if etc.Error == "" {
-				t.Errorf("Error in test case %s when there should not be.", etc.Description)
+				t.Errorf("unexpected error in test case %s: %s", etc.Description, err)
 			}
 			if !strings.Contains(message, etc.Error) {
 				t.Errorf("error in test case:%s:%s\n", etc.Description, message)
@@ -270,42 +279,41 @@ func TestExpandTemplate(t *testing.T) {
 	}
 }
 
-func getValidResponse(t *testing.T, description string) *ExpandedTemplate {
-	response, err := validResponseTestCaseData.Unmarshal()
-	if err != nil {
-		t.Errorf("cannot unmarshal valid response for test case '%s': %s\n", description, err)
+func getValidServiceResponse() *common.Configuration {
+	conf := &common.Configuration{}
+	if err := yaml.Unmarshal(validResponseTestCaseData, conf); err != nil {
+		panic(fmt.Errorf("cannot unmarshal valid response: %s\n", err))
 	}
 
-	return response
+	return conf
 }
 
-func expanderErrorHandler(w http.ResponseWriter, r *http.Request) {
-	defer r.Body.Close()
-	http.Error(w, "something failed", http.StatusInternalServerError)
-}
+func getValidExpandedConfiguration() *ExpandedConfiguration {
+	conf := getValidServiceResponse()
+	layout := &common.Layout{Resources: []*common.LayoutResource{}}
+	return &ExpandedConfiguration{Config: conf, Layout: layout}
 
-var roundTripResponse = ExpansionResponse{
-	Config: roundTripExpanded,
-	Layout: roundTripLayout,
 }
 
-var roundTripResponse2 = ExpansionResponse{
-	Config: roundTripExpanded2,
-	Layout: roundTripLayout2,
-}
-
-var roundTripResponses = []ExpansionResponse{
-	roundTripResponse,
-	roundTripResponse2,
+func expanderErrorHandler(w http.ResponseWriter, r *http.Request) {
+	defer r.Body.Close()
+	http.Error(w, "simulated failure", http.StatusInternalServerError)
 }
 
+/*
 func roundTripHandler(w http.ResponseWriter, r *http.Request) {
 	defer r.Body.Close()
 	handler := "expandybird: expand"
 	util.LogHandlerEntry(handler, r)
+	if len(roundTripResponses) < 1 {
+		http.Error(w, "Too many calls to round trip handler", http.StatusInternalServerError)
+		return
+	}
+
 	util.LogHandlerExitWithJSON(handler, w, roundTripResponses[0], http.StatusOK)
 	roundTripResponses = roundTripResponses[1:]
 }
+*/
 
 func expanderSuccessHandler(w http.ResponseWriter, r *http.Request) {
 	handler := "expandybird: expand"
@@ -318,19 +326,22 @@ func expanderSuccessHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	template := &common.Template{}
-	if err := json.Unmarshal(body, template); err != nil {
+	svcReq := &expansion.ServiceRequest{}
+	if err := json.Unmarshal(body, svcReq); err != nil {
 		status := fmt.Sprintf("cannot unmarshal request body:%s\n%s\n", err, body)
 		http.Error(w, status, http.StatusInternalServerError)
 		return
 	}
 
-	if !reflect.DeepEqual(validTemplateTestCaseData, *template) {
+	/*
+		if !reflect.DeepEqual(validRequestTestCaseData, *svcReq) {
 			status := fmt.Sprintf("error in http handler:\nwant:%s\nhave:%s\n",
-			util.ToJSONOrError(validTemplateTestCaseData), util.ToJSONOrError(template))
+				util.ToJSONOrError(validRequestTestCaseData), util.ToJSONOrError(template))
 			http.Error(w, status, http.StatusInternalServerError)
 			return
 		}
+	*/
 
-	util.LogHandlerExitWithJSON(handler, w, validResponseTestCaseData, http.StatusOK)
+	svcResp := getValidServiceResponse()
+	util.LogHandlerExitWithJSON(handler, w, svcResp, http.StatusOK)
 }