First pass removal of TypeResolver

cmd/manager/manager/expander.go

@@ -43,8 +43,17 @@ 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
+}
+
 // NewExpander returns a new initialized Expander.
-func NewExpander(url string, tr TypeResolver) Expander {
+func NewExpander(url string) Expander {
+	tr := &mockResolver{}
 	return &expander{url, tr}
 }
 
@@ -53,6 +62,12 @@ type expander struct {
 	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)
 }

cmd/manager/manager/expander_test.go

@@ -32,21 +32,6 @@ import (
 	"github.com/ghodss/yaml"
 )
 
-type mockResolver struct {
-	responses [][]*common.ImportFile
-	t         *testing.T
-}
-
-func (r *mockResolver) ResolveTypes(c *common.Configuration, i []*common.ImportFile) ([]*common.ImportFile, error) {
-	if len(r.responses) < 1 {
-		return nil, nil
-	}
-
-	ret := r.responses[0]
-	r.responses = r.responses[1:]
-	return ret, nil
-}
-
 var validTemplateTestCaseData = common.Template{
 	Name:    "TestTemplate",
 	Content: string(validContentTestCaseData),
@@ -232,7 +217,6 @@ type ExpanderTestCase struct {
 	Description   string
 	Error         string
 	Handler       func(w http.ResponseWriter, r *http.Request)
-	Resolver      TypeResolver
 	ValidResponse *ExpandedTemplate
 }
 
@@ -247,33 +231,21 @@ func TestExpandTemplate(t *testing.T) {
 			"expect success for ExpandTemplate",
 			"",
 			expanderSuccessHandler,
-			&mockResolver{},
 			getValidResponse(t, "expect success for ExpandTemplate"),
 		},
 		{
 			"expect error for ExpandTemplate",
 			"cannot expand template",
 			expanderErrorHandler,
-			&mockResolver{},
 			nil,
 		},
-		{
-			"expect success for ExpandTemplate with two expansions",
-			"",
-			roundTripHandler,
-			&mockResolver{[][]*common.ImportFile{
-				{},
-				{&common.ImportFile{Name: "test.py"}},
-			}, t},
-			roundTripResponse,
-		},
 	}
 
 	for _, etc := range tests {
 		ts := httptest.NewServer(http.HandlerFunc(etc.Handler))
 		defer ts.Close()
 
-		expander := NewExpander(ts.URL, etc.Resolver)
+		expander := NewExpander(ts.URL)
 		actualResponse, err := expander.ExpandTemplate(&validTemplateTestCaseData)
 		if err != nil {
 			message := err.Error()

cmd/manager/manager/typeresolver.go

@@ -1,245 +0,0 @@
-/*
-Copyright 2015 The Kubernetes Authors All rights reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package manager
-
-import (
-	"fmt"
-	"net/http"
-
-	"github.com/kubernetes/helm/pkg/common"
-	"github.com/kubernetes/helm/pkg/registry"
-	"github.com/kubernetes/helm/pkg/util"
-
-	"github.com/ghodss/yaml"
-)
-
-const (
-	maxURLImports = 100
-	schemaSuffix  = ".schema"
-)
-
-// 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)
-}
-
-type typeResolver struct {
-	maxUrls int
-	rp      registry.RegistryProvider
-	c       util.HTTPClient
-}
-
-type fetchableURL struct {
-	reg registry.Registry
-	url string
-}
-
-type fetchUnit struct {
-	urls []fetchableURL
-}
-
-// NewTypeResolver returns a new initialized TypeResolver.
-func NewTypeResolver(rp registry.RegistryProvider, c util.HTTPClient) TypeResolver {
-	return &typeResolver{maxUrls: maxURLImports, rp: rp, c: c}
-}
-
-func resolverError(c *common.Configuration, err error) error {
-	return fmt.Errorf("cannot resolve types in configuration %s due to: \n%s\n",
-		c, err)
-}
-
-func (tr *typeResolver) performHTTPGet(d util.HTTPDoer, u string, allowMissing bool) (content string, err error) {
-	g := tr.c
-	if d != nil {
-		g = util.NewHTTPClient(3, d, util.NewSleeper())
-	}
-	r, code, err := g.Get(u)
-	if err != nil {
-		return "", err
-	}
-
-	if allowMissing && code == http.StatusNotFound {
-		return "", nil
-	}
-
-	if code != http.StatusOK {
-		return "", fmt.Errorf(
-			"Received status code %d attempting to fetch Type at %s", code, u)
-	}
-
-	return r, nil
-}
-
-// ResolveTypes resolves the types in the supplied configuration and returns
-// resolved type definitions in t.ImportFiles. Types can be either
-// primitive (i.e., built in), resolved (i.e., already t.ImportFiles), or remote
-// (i.e., described by a URL that must be fetched to resolve the type).
-func (tr *typeResolver) ResolveTypes(config *common.Configuration, imports []*common.ImportFile) ([]*common.ImportFile, error) {
-	existing := map[string]bool{}
-	for _, v := range imports {
-		existing[v.Name] = true
-	}
-
-	fetched := map[string][]*common.ImportFile{}
-	// TODO(vaikas): Need to account for multiple URLs being fetched for a given type.
-	toFetch := make([]*fetchUnit, 0, tr.maxUrls)
-	for _, r := range config.Resources {
-		// Map the type to a fetchable URL (if applicable) or skip it if it's a non-fetchable type (primitive for example).
-		urls, urlRegistry, err := registry.GetDownloadURLs(tr.rp, r.Type)
-		if err != nil {
-			return nil, resolverError(config, fmt.Errorf("Failed to understand download url for %s: %v", r.Type, err))
-		}
-		if !existing[r.Type] {
-			f := &fetchUnit{}
-			for _, u := range urls {
-				if len(u) > 0 {
-					f.urls = append(f.urls, fetchableURL{urlRegistry, u})
-					// Add to existing map so it is not fetched multiple times.
-					existing[r.Type] = true
-				}
-			}
-			if len(f.urls) > 0 {
-				toFetch = append(toFetch, f)
-				fetched[f.urls[0].url] = append(fetched[f.urls[0].url], &common.ImportFile{Name: r.Type, Path: f.urls[0].url})
-			}
-		}
-	}
-
-	count := 0
-	for len(toFetch) > 0 {
-		// 1. If short github URL, resolve to a download URL
-		// 2. Fetch import URL. Exit if no URLs left
-		// 3. Check/handle HTTP status
-		// 4. Store results in all ImportFiles from that URL
-		// 5. Check for the optional schema file at import URL + .schema
-		// 6. Repeat 2,3 for schema file
-		// 7. Add each schema import to fetch if not already done
-		// 8. Mark URL done. Return to 1.
-		if count >= tr.maxUrls {
-			return nil, resolverError(config,
-				fmt.Errorf("Number of imports exceeds maximum of %d", tr.maxUrls))
-		}
-
-		templates := []string{}
-		url := toFetch[0].urls[0]
-		for _, u := range toFetch[0].urls {
-			template, err := tr.performHTTPGet(u.reg, u.url, false)
-			if err != nil {
-				return nil, resolverError(config, err)
-			}
-			templates = append(templates, template)
-		}
-
-		for _, i := range fetched[url.url] {
-			template, err := parseContent(templates)
-			if err != nil {
-				return nil, resolverError(config, err)
-			}
-			i.Content = template
-		}
-
-		schemaURL := url.url + schemaSuffix
-		sch, err := tr.performHTTPGet(url.reg, schemaURL, true)
-		if err != nil {
-			return nil, resolverError(config, err)
-		}
-
-		if sch != "" {
-			var s common.Schema
-			if err := yaml.Unmarshal([]byte(sch), &s); err != nil {
-				return nil, resolverError(config, err)
-			}
-			// Here we handle any nested imports in the schema we've just fetched.
-			for _, v := range s.Imports {
-				i := &common.ImportFile{Name: v.Name}
-				var existingSchema string
-				urls, urlRegistry, conversionErr := registry.GetDownloadURLs(tr.rp, v.Path)
-				if conversionErr != nil {
-					return nil, resolverError(config, fmt.Errorf("Failed to understand download url for %s: %v", v.Path, conversionErr))
-				}
-				if len(urls) == 0 {
-					// If it's not a fetchable URL, we need to use the type name as is, since it is a short name
-					// for a schema.
-					urls = []string{v.Path}
-				}
-				for _, u := range urls {
-					if len(fetched[u]) == 0 {
-						// If this import URL is new to us, add it to the URLs to fetch.
-						toFetch = append(toFetch, &fetchUnit{[]fetchableURL{{urlRegistry, u}}})
-					} else {
-						// If this is not a new import URL and we've already fetched its contents,
-						// reuse them. Also, check if we also found a schema for that import URL and
-						// record those contents for re-use as well.
-						if fetched[u][0].Content != "" {
-							i.Content = fetched[u][0].Content
-							if len(fetched[u+schemaSuffix]) > 0 {
-								existingSchema = fetched[u+schemaSuffix][0].Content
-							}
-						}
-					}
-					fetched[u] = append(fetched[u], i)
-					if existingSchema != "" {
-						fetched[u+schemaSuffix] = append(fetched[u+schemaSuffix],
-							&common.ImportFile{Name: v.Name + schemaSuffix, Content: existingSchema})
-					}
-				}
-			}
-
-			// Add the schema we've fetched as the schema for any templates which used this URL.
-			for _, i := range fetched[url.url] {
-				schemaImportName := i.Name + schemaSuffix
-				fetched[schemaURL] = append(fetched[schemaURL],
-					&common.ImportFile{Name: schemaImportName, Content: sch})
-			}
-		}
-
-		count = count + 1
-		toFetch = toFetch[1:]
-	}
-
-	ret := []*common.ImportFile{}
-	for _, v := range fetched {
-		ret = append(ret, v...)
-	}
-
-	return ret, nil
-}
-
-func parseContent(templates []string) (string, error) {
-	if len(templates) == 1 {
-		return templates[0], nil
-	}
-
-	// If there are multiple URLs that need to be fetched, that implies it's a package
-	// of raw Kubernetes objects. We need to fetch them all as a unit and create a
-	// template representing a package out of that below.
-	fakeConfig := &common.Configuration{}
-	for _, template := range templates {
-		o, err := util.ParseKubernetesObject([]byte(template))
-		if err != nil {
-			return "", fmt.Errorf("not a kubernetes object: %+v", template)
-		}
-		// Looks like a native Kubernetes object, create a configuration out of it
-		fakeConfig.Resources = append(fakeConfig.Resources, o)
-	}
-	marshalled, err := yaml.Marshal(fakeConfig)
-	if err != nil {
-		return "", fmt.Errorf("Failed to marshal: %+v", fakeConfig)
-	}
-	return string(marshalled), nil
-}

cmd/manager/manager/typeresolver_test.go

@@ -1,328 +0,0 @@
-/*
-Copyright 2015 The Kubernetes Authors All rights reserved.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package manager
-
-import (
-	"errors"
-	"net/http"
-	"reflect"
-	"strings"
-	"testing"
-
-	"github.com/ghodss/yaml"
-	"github.com/kubernetes/helm/pkg/common"
-	"github.com/kubernetes/helm/pkg/registry"
-)
-
-type responseAndError struct {
-	err  error
-	code int
-	resp string
-}
-
-type resolverTestCase struct {
-	config           string
-	imports          []*common.ImportFile
-	responses        map[string]responseAndError
-	urlcount         int
-	expectedErr      error
-	importOut        []*common.ImportFile
-	registryProvider registry.RegistryProvider
-}
-
-type testGetter struct {
-	responses map[string]responseAndError
-	count     int
-	test      *testing.T
-}
-
-var count = 0
-
-func (tg testGetter) Get(url string) (body string, code int, err error) {
-	count = count + 1
-	ret := tg.responses[url]
-	return ret.resp, ret.code, ret.err
-}
-
-func testDriver(c resolverTestCase, t *testing.T) {
-	g := &testGetter{test: t, responses: c.responses}
-	count = 0
-	r := &typeResolver{
-		maxUrls: 5,
-		rp:      c.registryProvider,
-		c:       g,
-	}
-
-	conf := &common.Configuration{}
-	dataErr := yaml.Unmarshal([]byte(c.config), conf)
-	if dataErr != nil {
-		panic("bad test data")
-	}
-
-	result, err := r.ResolveTypes(conf, c.imports)
-
-	if count != c.urlcount {
-		t.Errorf("Expected %d url GETs but only %d found %#v", c.urlcount, g.count, g)
-	}
-
-	if (err != nil && c.expectedErr == nil) || (err == nil && c.expectedErr != nil) {
-		t.Errorf("Expected error %s but found %s", c.expectedErr, err)
-	} else if err != nil && !strings.Contains(err.Error(), c.expectedErr.Error()) {
-		t.Errorf("Expected error %s but found %s", c.expectedErr, err)
-	}
-
-	resultImport := map[common.ImportFile]bool{}
-	expectedImport := map[common.ImportFile]bool{}
-	for _, i := range result {
-		resultImport[*i] = true
-	}
-
-	for _, i := range c.importOut {
-		expectedImport[*i] = true
-	}
-
-	if !reflect.DeepEqual(resultImport, expectedImport) {
-		t.Errorf("Expected imports %+v but found %+v", expectedImport, resultImport)
-	}
-}
-
-var simpleContent = `
-resources:
-- name: test
-  type: ReplicationController
-`
-
-func TestNoImports(t *testing.T) {
-	test := resolverTestCase{config: simpleContent}
-	testDriver(test, t)
-}
-
-var includeImport = `
-resources:
-- name: foo
-  type: foo.py
-`
-
-func TestIncludedImport(t *testing.T) {
-	imports := []*common.ImportFile{{Name: "foo.py"}}
-	test := resolverTestCase{
-		config:  includeImport,
-		imports: imports,
-	}
-	testDriver(test, t)
-}
-
-var templateSingleURL = `
-resources:
-- name: foo
-  type: http://my-fake-url
-`
-
-func TestSingleUrl(t *testing.T) {
-	finalImports := []*common.ImportFile{{Name: "http://my-fake-url", Path: "http://my-fake-url", Content: "my-content"}}
-
-	responses := map[string]responseAndError{
-		"http://my-fake-url":        {nil, http.StatusOK, "my-content"},
-		"http://my-fake-url.schema": {nil, http.StatusNotFound, ""},
-	}
-
-	test := resolverTestCase{
-		config:    templateSingleURL,
-		importOut: finalImports,
-		urlcount:  2,
-		responses: responses,
-	}
-	testDriver(test, t)
-}
-
-func TestSingleUrlWith500(t *testing.T) {
-	responses := map[string]responseAndError{
-		"http://my-fake-url": {nil, http.StatusInternalServerError, "my-content"},
-	}
-
-	test := resolverTestCase{
-		config:      templateSingleURL,
-		urlcount:    1,
-		responses:   responses,
-		expectedErr: errors.New("Received status code 500"),
-	}
-	testDriver(test, t)
-}
-
-var schema1 = `
-imports:
-- path: my-next-url
-  name: schema-import
-`
-
-func TestSingleUrlWithSchema(t *testing.T) {
-	finalImports := []*common.ImportFile{
-		{Name: "http://my-fake-url", Path: "http://my-fake-url", Content: "my-content"},
-		{Name: "schema-import", Content: "schema-import"},
-		{Name: "http://my-fake-url.schema", Content: schema1},
-	}
-
-	responses := map[string]responseAndError{
-		"http://my-fake-url":        {nil, http.StatusOK, "my-content"},
-		"http://my-fake-url.schema": {nil, http.StatusOK, schema1},
-		"my-next-url":               {nil, http.StatusOK, "schema-import"},
-		"my-next-url.schema":        {nil, http.StatusNotFound, ""},
-	}
-
-	test := resolverTestCase{
-		config:    templateSingleURL,
-		importOut: finalImports,
-		urlcount:  4,
-		responses: responses,
-	}
-	testDriver(test, t)
-}
-
-var templateExceedsMax = `
-resources:
-- name: foo
-  type: http://my-fake-url
-- name: foo1
-  type: http://my-fake-url1
-- name: foo2
-  type: http://my-fake-url2
-- name: foo3
-  type: http://my-fake-url3
-- name: foo4
-  type: http://my-fake-url4
-- name: foo5
-  type: http://my-fake-url5
-`
-
-func TestTooManyImports(t *testing.T) {
-	responses := map[string]responseAndError{
-		"http://my-fake-url":         {nil, http.StatusOK, "my-content"},
-		"http://my-fake-url.schema":  {nil, http.StatusNotFound, ""},
-		"http://my-fake-url1":        {nil, http.StatusOK, "my-content"},
-		"http://my-fake-url1.schema": {nil, http.StatusNotFound, ""},
-		"http://my-fake-url2":        {nil, http.StatusOK, "my-content"},
-		"http://my-fake-url2.schema": {nil, http.StatusNotFound, ""},
-		"http://my-fake-url3":        {nil, http.StatusOK, "my-content"},
-		"http://my-fake-url3.schema": {nil, http.StatusNotFound, ""},
-		"http://my-fake-url4":        {nil, http.StatusOK, "my-content"},
-		"http://my-fake-url4.schema": {nil, http.StatusNotFound, ""},
-		"http://my-fake-url5":        {nil, http.StatusOK, "my-content"},
-		"http://my-fake-url5.schema": {nil, http.StatusNotFound, ""},
-	}
-
-	test := resolverTestCase{
-		config:      templateExceedsMax,
-		urlcount:    10,
-		responses:   responses,
-		expectedErr: errors.New("Number of imports exceeds maximum of 5"),
-	}
-
-	testDriver(test, t)
-}
-
-var templateSharesImport = `
-resources:
-- name: foo
-  type: http://my-fake-url
-- name: foo1
-  type: http://my-fake-url1
-`
-
-var schema2 = `
-imports:
-- path: my-next-url
-  name: schema-import-1
-`
-
-func TestSharedImport(t *testing.T) {
-	finalImports := []*common.ImportFile{
-		{Name: "http://my-fake-url", Path: "http://my-fake-url", Content: "my-content"},
-		{Name: "http://my-fake-url1", Path: "http://my-fake-url1", Content: "my-content-1"},
-		{Name: "schema-import", Content: "schema-import"},
-		{Name: "schema-import-1", Content: "schema-import"},
-		{Name: "http://my-fake-url.schema", Content: schema1},
-		{Name: "http://my-fake-url1.schema", Content: schema2},
-	}
-
-	responses := map[string]responseAndError{
-		"http://my-fake-url":         {nil, http.StatusOK, "my-content"},
-		"http://my-fake-url.schema":  {nil, http.StatusOK, schema1},
-		"http://my-fake-url1":        {nil, http.StatusOK, "my-content-1"},
-		"http://my-fake-url1.schema": {nil, http.StatusOK, schema2},
-		"my-next-url":                {nil, http.StatusOK, "schema-import"},
-		"my-next-url.schema":         {nil, http.StatusNotFound, ""},
-	}
-
-	test := resolverTestCase{
-		config:    templateSharesImport,
-		urlcount:  6,
-		responses: responses,
-		importOut: finalImports,
-	}
-
-	testDriver(test, t)
-}
-
-var templateShortGithubTemplate = `
-resources:
-- name: foo
-  type: github.com/kubernetes/application-dm-templates/common/replicatedservice:v1
-- name: foo1
-  type: github.com/kubernetes/application-dm-templates/common/replicatedservice:v2
-`
-
-func TestShortGithubUrl(t *testing.T) {
-	finalImports := []*common.ImportFile{
-		{
-			Name:    "github.com/kubernetes/application-dm-templates/common/replicatedservice:v1",
-			Path:    "https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v1/replicatedservice.py",
-			Content: "my-content"},
-		{
-			Name:    "github.com/kubernetes/application-dm-templates/common/replicatedservice:v2",
-			Path:    "https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v2/replicatedservice.py",
-			Content: "my-content-2"},
-	}
-
-	downloadResponses := map[string]registry.DownloadResponse{
-		"https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v1/replicatedservice.py":        {Err: nil, Code: http.StatusOK, Body: "my-content"},
-		"https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v1/replicatedservice.py.schema": {Err: nil, Code: http.StatusNotFound, Body: ""},
-		"https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v2/replicatedservice.py":        {Err: nil, Code: http.StatusOK, Body: "my-content-2"},
-		"https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v2/replicatedservice.py.schema": {Err: nil, Code: http.StatusNotFound, Body: ""},
-	}
-
-	githubURLMaps := map[registry.Type]registry.TestURLAndError{
-		registry.NewTypeOrDie("common", "replicatedservice", "v1"): {URL: "https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v1/replicatedservice.py", Err: nil},
-		registry.NewTypeOrDie("common", "replicatedservice", "v2"): {URL: "https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v2/replicatedservice.py", Err: nil},
-	}
-
-	gcsURLMaps := map[registry.Type]registry.TestURLAndError{
-		registry.NewTypeOrDie("common", "replicatedservice", "v1"): {URL: "https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v1/replicatedservice.py", Err: nil},
-		registry.NewTypeOrDie("common", "replicatedservice", "v2"): {URL: "https://raw.githubusercontent.com/kubernetes/application-dm-templates/master/common/replicatedservice/v2/replicatedservice.py", Err: nil},
-	}
-
-	grp := registry.NewTestGithubRegistryProviderWithDownloads("github.com/kubernetes/application-dm-templates", githubURLMaps, downloadResponses)
-	gcsrp := registry.NewTestGCSRegistryProvider("gs://charts", gcsURLMaps)
-	test := resolverTestCase{
-		config:           templateShortGithubTemplate,
-		importOut:        finalImports,
-		urlcount:         0,
-		responses:        map[string]responseAndError{},
-		registryProvider: registry.NewRegistryProvider(nil, grp, gcsrp, registry.NewInmemCredentialProvider()),
-	}
-
-	testDriver(test, t)
-}