Do not publish untagged or latest to mixin feed (#1466)

* Do not publish untagged or latest to mixin feed

The file structure in the bin is used when generating the mixin feed. So
if there is a folder named "latest" then a version "latest" is put in
the atom.xml. A previous commit caused an untagged release to be in the
bin, instead of being renamed to canary and uploaded to our feed which
broke installing porter.

I have moved the directory prep into mage, calling it in make xbuild-all
so that it is guaranteed to happen. I have also added a check in the
atom feed generation to ignore those releases if found.

Now after you cross build, your bin looks like this

bin/
  mixins/
    exec/
      canary/... OR
      v1.2.3/...

Signed-off-by: Carolyn Van Slyck <[email protected]>

* Fix typos

Signed-off-by: Carolyn Van Slyck <[email protected]>

* Make most recent build available from constant location

Signed-off-by: Carolyn Van Slyck <[email protected]>

* Only generate for canary and tagged versions

* Flip the logic for the atom feed generation to ignore anything that
isn't canary or vX.Y.Z, this lets us put whatever we want in bin without
worry that it will end up in the feed.
* When we save the xbuild-all binaries, save them to bin/dev instead of
bin/latest to avoid confusion
* Found another packr directory that needed to be cleaned after build

Signed-off-by: Carolyn Van Slyck <[email protected]>

* Include status text when mixin install fails

Signed-off-by: Carolyn Van Slyck <[email protected]>

Makefile

@@ -217,6 +217,7 @@ clean-last-testrun: stop-local-docker-registry
 clean-packr: packr2
 	cd cmd/porter && packr2 clean
 	cd pkg/porter && packr2 clean
+	cd pkg/pkgmgmt/feed && packr2 clean
 	$(foreach MIXIN, $(INT_MIXINS), \
 		`cd pkg/$(MIXIN) && packr2 clean`; \
 	)

mage/releases/publish.go

@@ -3,6 +3,8 @@ package releases
 import (
 	"context"
 	"encoding/json"
+	"log"
+	"os"
 	"path"
 	"path/filepath"
 	"strings"
@@ -23,27 +25,56 @@ const (
 	StaticCache   = "max-age=604800" // 1 week
 )
 
-// Publish a mixin's binaries.
-func PublishMixin(mixin string, version string, permalink string) {
+// Prepares bin directory for publishing
+func PrepareMixinForPublish(mixin string, version string, permalink string) {
+	// Prepare the bin directory for generating a mixin feed
+	// We want the bin to contain either a version directory (v1.2.3) or a canary directory.
+	// We do not want a latest directory, latest entries are calculated using the most recent
+	// timestamp in the atom.xml, not from an explicit entry.
 	binDir := filepath.Join("bin/mixins/", mixin)
+	// Temp hack until we have mixin.mk totally moved into mage
+	if mixin == "porter" {
+		binDir = "bin"
+	}
 	versionDir := filepath.Join(binDir, version)
+	permalinkDir := filepath.Join(binDir, permalink)
+
+	mgx.Must(os.RemoveAll(permalinkDir))
+	log.Printf("mv %s %s\n", versionDir, permalinkDir)
+	mgx.Must(os.Rename(versionDir, permalinkDir))
+}
+
+// Publish a mixin's binaries.
+func PublishMixin(mixin string, version string, permalink string) {
+	var publishDir string
+	if permalink == "canary" {
+		publishDir = filepath.Join("bin/mixins/", mixin, permalink)
+	} else {
+		publishDir = filepath.Join("bin/mixins/", mixin, version)
+	}
+
 	if permalink == "latest" {
-		must.RunV("az", "storage", "blob", "upload-batch", "-d", path.Join(ContainerName, "mixins", mixin, version), "-s", versionDir, "--content-cache-control", StaticCache)
+		must.RunV("az", "storage", "blob", "upload-batch", "-d", path.Join(ContainerName, "mixins", mixin, version), "-s", publishDir, "--content-cache-control", StaticCache)
 	}
-	must.RunV("az", "storage", "blob", "upload-batch", "-d", path.Join(ContainerName, "mixins", mixin, permalink), "-s", versionDir, "--content-cache-control", VolatileCache)
+	must.RunV("az", "storage", "blob", "upload-batch", "-d", path.Join(ContainerName, "mixins", mixin, permalink), "-s", publishDir, "--content-cache-control", VolatileCache)
 }
 
-// Generate an updated mixin feed and releases it.
+// Generate an updated mixin feed and publishes it.
 func PublishMixinFeed(ctx context.Context) {
 	leaseId, unlock, err := lockMixinFeed(ctx)
 	defer unlock()
 	mgx.Must(err)
 
 	must.RunE("az", "storage", "blob", "download", "-c", ContainerName, "-n", mixinFeedBlob, "-f", mixinFeedFile, "--lease-id", leaseId)
-	must.RunV("bin/porter", "mixins", "feed", "generate", "-d", filepath.Dir(mixinFeedFile), "-f", mixinFeedFile, "-t", "build/atom-template.xml")
+	GenerateMixinFeed()
 	must.RunV("az", "storage", "blob", "upload", "-c", ContainerName, "-n", mixinFeedBlob, "-f", mixinFeedFile, "--content-cache-control", VolatileCache, "--lease-id", leaseId)
 }
 
+// Generate a mixin feed from any mixin versions in bin.
+func GenerateMixinFeed() {
+	must.RunV("bin/porter", "mixins", "feed", "generate", "-d", filepath.Dir(mixinFeedFile), "-f", mixinFeedFile, "-t", "build/atom-template.xml")
+}
+
 // Tries to get a lock on the mixin feed in blob storage, returning the lease id
 func lockMixinFeed(ctx context.Context) (string, func(), error) {
 	var leaseJson string

magefile.go

@@ -139,10 +139,10 @@ func UseXBuildBinaries() error {
 	}
 
 	copies := map[string]string{
-		"bin/latest/porter-$GOOS-amd64$EXT":           "bin/porter$EXT",
-		"bin/latest/porter-linux-amd64":               "bin/runtimes/porter-runtime",
-		"bin/mixins/exec/latest/exec-$GOOS-amd64$EXT": "bin/mixins/exec/exec$EXT",
-		"bin/mixins/exec/latest/exec-linux-amd64":     "bin/mixins/exec/runtimes/exec-runtime",
+		"bin/dev/porter-$GOOS-amd64$EXT":           "bin/porter$EXT",
+		"bin/dev/porter-linux-amd64":               "bin/runtimes/porter-runtime",
+		"bin/mixins/exec/dev/exec-$GOOS-amd64$EXT": "bin/mixins/exec/exec$EXT",
+		"bin/mixins/exec/dev/exec-linux-amd64":     "bin/mixins/exec/runtimes/exec-runtime",
 	}
 
 	r := strings.NewReplacer("$GOOS", goos, "$EXT", ext, "$PWD", pwd)

mixin.mk

@@ -45,13 +45,14 @@ xbuild-all:
 		$(foreach ARCH, $(SUPPORTED_ARCHES), \
 				$(MAKE) $(MAKE_OPTS) CLIENT_PLATFORM=$(OS) CLIENT_ARCH=$(ARCH) MIXIN=$(MIXIN) xbuild -f mixin.mk; \
 		))
-	cp -R $(BINDIR)/$(VERSION) $(BINDIR)/latest
+	@# Copy most recent build into bin/dev so that subsequent build steps can easily find it, not used for publishing
+	cp -R $(BINDIR)/$(VERSION) $(BINDIR)/dev
+	mage PrepareMixinForPublish $(MIXIN) $(VERSION) $(PERMALINK)
 
 xbuild: $(BINDIR)/$(VERSION)/$(MIXIN)-$(CLIENT_PLATFORM)-$(CLIENT_ARCH)$(FILE_EXT)
 $(BINDIR)/$(VERSION)/$(MIXIN)-$(CLIENT_PLATFORM)-$(CLIENT_ARCH)$(FILE_EXT):
 	mkdir -p $(dir $@)
 	GOOS=$(CLIENT_PLATFORM) GOARCH=$(CLIENT_ARCH) $(XBUILD) -o $@ ./cmd/$(MIXIN)
 
 clean:
-	-rm -r $(BINDIR)/latest
 	-rm -fr bin/mixins/$(MIXIN)

pkg/pkgmgmt/client/install.go

@@ -164,7 +164,7 @@ func (fs *FileSystem) downloadFile(url url.URL, destPath string, executable bool
 		return errors.Wrapf(err, "error downloading %s", url.String())
 	}
 	if resp.StatusCode != 200 {
-		return errors.Errorf("bad status returned when downloading %s (%d)", url.String(), resp.StatusCode)
+		return errors.Errorf("bad status returned when downloading %s (%d) %s", url.String(), resp.StatusCode, resp.Status)
 	}
 	defer resp.Body.Close()
 

pkg/pkgmgmt/feed/generate.go

@@ -69,6 +69,17 @@ func (feed *MixinFeed) Generate(opts GenerateOptions) error {
 		matches := mixinRegex.FindStringSubmatch(path)
 		if len(matches) > 0 {
 			version := matches[2]
+
+			// As a safety measure, skip versions that shouldn't be put in the feed, we only want canary and tagged releases.
+			if version != "canary" {
+				versionRegex := regexp.MustCompile(`v\d+\.\d+\.\d+(-\d+-g[a-z0-9]{8})?`)
+				matches := versionRegex.FindStringSubmatch(version)
+				if len(matches) == 0 || // e.g. latest
+					len(matches) == 2 && matches[1] != "" { // e.g. v1.2.3-2-g12345678
+					return nil
+				}
+			}
+
 			mixin := matches[3]
 			filename := info.Name()
 			updated := info.ModTime()

pkg/pkgmgmt/feed/generate_test.go

@@ -54,6 +54,15 @@ func TestGenerate(t *testing.T) {
 	tc.FileSystem.Chtimes("bin/canary/exec-linux-amd64", up10, up10)
 	tc.FileSystem.Chtimes("bin/canary/exec-windows-amd64.exe", up10, up10)
 
+	// Create extraneous release directories that should be ignored
+	tc.FileSystem.Create("bin/v0.34.0-4-gd8ec3bbf/helm-darwin-amd64")
+	tc.FileSystem.Create("bin/v0.34.0-4-gd8ec3bbf/helm-linux-amd64")
+	tc.FileSystem.Create("bin/v0.34.0-4-gd8ec3bbf/helm-windows-amd64.exe")
+
+	tc.FileSystem.Create("bin/latest/helm-darwin-amd64")
+	tc.FileSystem.Create("bin/latest/helm-linux-amd64")
+	tc.FileSystem.Create("bin/latest/helm-windows-amd64.exe")
+
 	opts := GenerateOptions{
 		AtomFile:        "atom.xml",
 		SearchDirectory: "bin",