Fit and finish from review

- extract meta type names as constants
- fix slice append gotcha: when creating a new slice, copy from old
  first (i.e. copyAndAppendMeta())
- various renames and inlining to improve clarity

hack/test-all-minikube-local-registry.sh

@@ -5,7 +5,8 @@ set -e -x -u
 export KBLD_E2E_DOCKERHUB_USERNAME=minikube-tests
 export KBLD_E2E_DOCKERHUB_HOSTNAME=$(minikube ip):30777
 # uncomment to disable stress tests
-# export KBLD_E2E_SKIP_STRESS_TESTS=true
+export KBLD_E2E_SKIP_STRESS_TESTS=true
+export KBLD_E2E_SKIP_WHEN_HTTP_REGISTRY=true
 
 kapp deploy -a reg -f test/e2e/assets/minikube-local-registry.yml -y
 

pkg/kbld/cmd/resolve.go

@@ -246,11 +246,9 @@ func (o *ResolveOptions) emitLockOutput(conf ctlconf.Conf, resolvedImages *Proce
 			},
 		}
 		for _, urlImagePair := range resolvedImages.All() {
-			anns := o.imgpkgLockAnnotations(urlImagePair)
-
 			iLock.Images = append(iLock.Images, lockconfig.ImageRef{
 				Image:       urlImagePair.Image.URL,
-				Annotations: anns,
+				Annotations: o.imgpkgLockAnnotations(urlImagePair),
 			})
 		}
 		return iLock.WriteToPath(o.ImgpkgLockOutput)
@@ -263,13 +261,12 @@ func (o *ResolveOptions) imgpkgLockAnnotations(i ProcessedImageItem) map[string]
 	anns := map[string]string{
 		ctlconf.ImagesLockKbldID: i.UnprocessedImageURL.URL,
 	}
-	imageMetas := i.Metas
-	if len(imageMetas) > 0 {
-		metaYaml, err := yaml.Marshal(imageMetas)
+	if len(i.Metas) > 0 {
+		bs, err := yaml.Marshal(i.Metas)
 		if err != nil {
 			return anns
 		}
-		anns[ctlconf.ImagesLockKbldMetas] = string(metaYaml)
+		anns[ctlconf.ImagesLockKbldMetas] = string(bs)
 	}
 
 	return anns

pkg/kbld/config/config.go

@@ -68,7 +68,7 @@ type ImageOverride struct {
 	NewImage     string                     `json:"newImage"`
 	Preresolved  bool                       `json:"preresolved,omitempty"`
 	TagSelection *versions.VersionSelection `json:"tagSelection,omitempty"`
-	ImageMetas   []Meta                     `json:",omitempty"`
+	ImageMetas   []Meta                     `json:"metas,omitempty"`
 }
 
 type ImageDestination struct {
@@ -169,7 +169,7 @@ func NewConfigFromImagesLock(res ctlres.Resource) (Config, error) {
 	overridesConfig := NewConfig()
 
 	for _, image := range imagesLock.Images {
-		imageMeta, err := metasHistory(image.Annotations[ImagesLockKbldMetas])
+		imgMeta, err := NewMetasFromString(image.Annotations[ImagesLockKbldMetas])
 		if err != nil {
 			return Config{}, fmt.Errorf("Unmarshaling %s as %s annotation:  %s", res.Description(), ImagesLockKbldMetas, err)
 		}
@@ -179,7 +179,7 @@ func NewConfigFromImagesLock(res ctlres.Resource) (Config, error) {
 			},
 			NewImage:    image.Image,
 			Preresolved: true,
-			ImageMetas:  imageMeta,
+			ImageMetas:  imgMeta,
 		}
 		overridesConfig.Overrides = append(overridesConfig.Overrides, iOverride)
 	}

pkg/kbld/config/image_meta.go

@@ -13,36 +13,44 @@ type Meta interface {
 	meta()
 }
 
-type ImageMeta struct {
+type imageMeta struct {
 	Metas []Meta
 }
 
+const (
+	GitMeta         = "git"
+	LocalMeta       = "local"
+	ResolvedMeta    = "resolved"
+	TaggedMeta      = "tagged"
+	PreresolvedMeta = "preresolved"
+)
+
 type BuiltImageSourceGit struct {
-	Type      string // always set to 'git'
+	Type      string // always set to GitMeta
 	RemoteURL string `json:",omitempty" yaml:",omitempty"`
 	SHA       string
 	Dirty     bool
 	Tags      []string `json:",omitempty" yaml:",omitempty"`
 }
 
 type BuiltImageSourceLocal struct {
-	Type string // always set to 'local'
+	Type string // always set to LocalMeta
 	Path string
 }
 
 type ResolvedImageSourceURL struct {
-	Type string // always set to 'resolved'
+	Type string // always set to ResolvedMeta
 	URL  string
 	Tag  string
 }
 
 type TaggedImageMeta struct {
-	Type string // always set to 'tagged'
+	Type string // always set to TaggedMeta
 	Tags []string
 }
 
 type PreresolvedImageSourceURL struct {
-	Type string // always set to 'preresolved'
+	Type string // always set to PreresolvedMeta
 	URL  string
 	Tag  string `json:",omitempty" yaml:",omitempty"`
 }
@@ -53,18 +61,18 @@ func (ResolvedImageSourceURL) meta()    {}
 func (TaggedImageMeta) meta()           {}
 func (PreresolvedImageSourceURL) meta() {}
 
-func metasHistory(metas string) ([]Meta, error) {
-	imageMeta := ImageMeta{}
-	err := yaml.Unmarshal([]byte(metas), &imageMeta)
+func NewMetasFromString(metas string) ([]Meta, error) {
+	imgMeta := imageMeta{}
+	err := yaml.Unmarshal([]byte(metas), &imgMeta)
 	if err != nil {
 		return []Meta{}, err
 	}
-	return imageMeta.Metas, nil
+	return imgMeta.Metas, nil
 }
 
-var _ json.Unmarshaler = &ImageMeta{}
+var _ json.Unmarshaler = &imageMeta{}
 
-func (m *ImageMeta) UnmarshalJSON(data []byte) error {
+func (m *imageMeta) UnmarshalJSON(data []byte) error {
 	var list []interface{}
 	err := yaml.Unmarshal(data, &list)
 	if err != nil {
@@ -81,16 +89,31 @@ func (m *ImageMeta) UnmarshalJSON(data []byte) error {
 		yamlItem, _ := yaml.Marshal(&item)
 
 		switch {
-		case yaml.Unmarshal(yamlItem, &local) == nil && local.Type == "local":
+		case yaml.Unmarshal(yamlItem, &local) == nil && local.Type == LocalMeta:
 			m.Metas = append(m.Metas, local)
-		case yaml.Unmarshal(yamlItem, &git) == nil && git.Type == "git":
+		case yaml.Unmarshal(yamlItem, &git) == nil && git.Type == GitMeta:
 			m.Metas = append(m.Metas, git)
-		case yaml.Unmarshal(yamlItem, &res) == nil && res.Type == "resolved":
+		case yaml.Unmarshal(yamlItem, &res) == nil && res.Type == ResolvedMeta:
 			m.Metas = append(m.Metas, res)
-		case yaml.Unmarshal(yamlItem, &preres) == nil && preres.Type == "preresolved":
+		case yaml.Unmarshal(yamlItem, &preres) == nil && preres.Type == PreresolvedMeta:
 			m.Metas = append(m.Metas, preres)
-		case yaml.Unmarshal(yamlItem, &tag) == nil && tag.Type == "tagged":
+		case yaml.Unmarshal(yamlItem, &tag) == nil && tag.Type == TaggedMeta:
 			m.Metas = append(m.Metas, tag)
+		default:
+			// ignore unknown meta.
+			// At this time...
+			// - "Meta" are provided as primarily optional diagnostic information
+			//   rather than operational data (read: less important). Losing
+			//   this information does not change the correctness of kbld's
+			//   primary purpose during deployment: to rewrite image references.
+			//   It would be more than an annoyance to error-out if we were
+			//   unable to parse such data.
+			// - Ideally, yes, we'd at least report a warning. However, if there's
+			//   a systemic condition (e.g. using an older version of kbld to
+			//   deploy than was used to package) there would likely be a flurry
+			//   of warnings. So, the feature would quickly need an enhancement
+			//   to de-dup such warnings. (read: added complexity)
+			// see also https://github.com/vmware-tanzu/carvel-kbld/issues/160
 		}
 	}
 	return nil

pkg/kbld/image/built.go

@@ -117,15 +117,15 @@ func (i BuiltImage) sources() ([]ctlconf.Meta, error) {
 	}
 
 	sources = append(sources, ctlconf.BuiltImageSourceLocal{
-		Type: "local",
+		Type: ctlconf.LocalMeta,
 		Path: absPath,
 	})
 
 	gitRepo := NewGitRepo(absPath)
 
 	if gitRepo.IsValid() {
 		var err error
-		git := ctlconf.BuiltImageSourceGit{Type: "git"}
+		git := ctlconf.BuiltImageSourceGit{Type: ctlconf.GitMeta}
 
 		git.RemoteURL, err = gitRepo.RemoteURL()
 		if err != nil {

pkg/kbld/image/preresolved.go

@@ -13,11 +13,16 @@ type PreresolvedImage struct {
 }
 
 func NewPreresolvedImage(url string, metas []ctlconf.Meta) PreresolvedImage {
-	return PreresolvedImage{url, metas}
+	return PreresolvedImage{url, copyAndAppendMeta(metas)}
 }
 
 func (i PreresolvedImage) URL() (string, []ctlconf.Meta, error) {
-	imageMetas := append(i.metas, ctlconf.PreresolvedImageSourceURL{Type: "preresolved", URL: i.url})
-
+	imageMetas := copyAndAppendMeta(i.metas, ctlconf.PreresolvedImageSourceURL{Type: ctlconf.PreresolvedMeta, URL: i.url})
 	return i.url, imageMetas, nil
 }
+
+func copyAndAppendMeta(existing []ctlconf.Meta, new ...ctlconf.Meta) []ctlconf.Meta {
+	all := make([]ctlconf.Meta, len(existing), len(existing)+len(new))
+	copy(all, existing)
+	return append(all, new...)
+}

pkg/kbld/image/resolved.go

@@ -49,7 +49,7 @@ func (i ResolvedImage) URL() (string, []ctlconf.Meta, error) {
 		return "", nil, err
 	}
 
-	metas = append(metas, ctlconf.ResolvedImageSourceURL{Type: "resolved", URL: i.url, Tag: tag.TagStr()})
+	metas = append(metas, ctlconf.ResolvedImageSourceURL{Type: ctlconf.ResolvedMeta, URL: i.url, Tag: tag.TagStr()})
 
 	return url, metas, nil
 }

pkg/kbld/image/tagged.go

@@ -46,7 +46,7 @@ func (i TaggedImage) URL() (string, []ctlconf.Meta, error) {
 			}
 		}
 
-		metas = append(metas, ctlconf.TaggedImageMeta{Type: "tagged", Tags: i.imgDst.Tags})
+		metas = append(metas, ctlconf.TaggedImageMeta{Type: ctlconf.TaggedMeta, Tags: i.imgDst.Tags})
 	}
 
 	return url, metas, err