fix(modules): properly persist channel resolve information (#139)

* fix(modules): properly persist channel resolve information

* document debug flag

* update error string

* use new devbase

* use newest rc devbase

cmd/stencil/stencil.go

@@ -108,6 +108,11 @@ func main() {
 			Name:  "allow-major-version-upgrades",
 			Usage: "Allow major version upgrades without confirmation",
 		},
+		&cli.BoolFlag{
+			Name:    "debug",
+			Usage:   "Enables debug logging for version resolution, template render, and other useful information",
+			Aliases: []string{"d"},
+		},
 		///EndBlock(flags)
 	}
 	app.Commands = []*cli.Command{

internal/cmd/stencil/stencil.go

@@ -104,7 +104,7 @@ func (c *Command) Run(ctx context.Context) error {
 	}
 
 	c.log.Info("Fetching dependencies")
-	mods, err := modules.GetModulesForService(ctx, c.token, c.manifest)
+	mods, err := modules.GetModulesForService(ctx, c.token, c.manifest, c.log)
 	if err != nil {
 		return errors.Wrap(err, "failed to process modules list")
 	}

internal/modules/module_test.go

@@ -6,13 +6,22 @@ package modules_test
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	"github.com/getoutreach/stencil/internal/modules"
 	"github.com/getoutreach/stencil/pkg/configuration"
+	"github.com/sirupsen/logrus"
 	"gotest.tools/v3/assert"
 )
 
+// newLogger creates a new logger for testing
+func newLogger() logrus.FieldLogger {
+	log := logrus.New()
+	log.SetLevel(logrus.DebugLevel)
+	return log
+}
+
 func TestCanFetchModule(t *testing.T) {
 	ctx := context.Background()
 	m, err := modules.New(ctx, "", &configuration.TemplateRepository{Name: "github.com/getoutreach/stencil-base", Version: "main"})
@@ -42,7 +51,7 @@ func TestReplacementLocalModule(t *testing.T) {
 		},
 	}
 
-	mods, err := modules.GetModulesForService(context.Background(), "", sm)
+	mods, err := modules.GetModulesForService(context.Background(), "", sm, newLogger())
 	assert.NilError(t, err, "expected GetModulesForService() to not error")
 	assert.Equal(t, len(mods), 1, "expected exactly one module to be returned")
 	assert.Equal(t, mods[0].URI, sm.Replacements["github.com/getoutreach/stencil-base"],
@@ -58,7 +67,7 @@ func TestCanGetLatestVersion(t *testing.T) {
 				Name: "github.com/getoutreach/stencil-base",
 			},
 		},
-	})
+	}, newLogger())
 	assert.NilError(t, err, "failed to call GetModulesForService()")
 	assert.Equal(t, len(mods), 1, "expected exactly one module to be returned")
 }
@@ -79,7 +88,7 @@ func TestHandleMultipleConstraints(t *testing.T) {
 		Replacements: map[string]string{
 			"nested_constraint": "file://testdata/nested_constraint",
 		},
-	})
+	}, newLogger())
 	assert.NilError(t, err, "failed to call GetModulesForService()")
 	assert.Equal(t, len(mods), 2, "expected exactly two modules to be returned")
 
@@ -101,7 +110,7 @@ func TestHandleNestedModules(t *testing.T) {
 			"a": "file://testdata/nested_modules/a",
 			"b": "file://testdata/nested_modules/b",
 		},
-	})
+	}, newLogger())
 	assert.NilError(t, err, "failed to call GetModulesForService()")
 	assert.Equal(t, len(mods), 2, "expected exactly two modules to be returned")
 	assert.Equal(t, mods[0].Name, "a", "expected module to match")
@@ -125,10 +134,10 @@ func TestFailOnIncompatibleConstraints(t *testing.T) {
 		Replacements: map[string]string{
 			"nested_constraint": "file://testdata/nested_constraint",
 		},
-	})
+	}, newLogger())
 	assert.Error(t, err,
 		//nolint:lll // Why: That's the error :(
-		"failed to resolve module 'github.com/getoutreach/stencil-base' with constraints\n└─ testing-service (top-level) wants >=0.5.0\n  └─ nested_constraint wants ~0.3.0\n: no version found matching criteria",
+		"failed to resolve module 'github.com/getoutreach/stencil-base' with constraints\n└─ testing-service (top-level) wants >=0.5.0\n  └─ nested_constraint@v0.0.0-+ wants ~0.3.0\n: no version found matching criteria",
 		"expected GetModulesForService() to error")
 }
 
@@ -143,7 +152,7 @@ func TestSupportChannelAndConstraint(t *testing.T) {
 				Version: "v0.6.0-rc.4",
 			},
 		},
-	})
+	}, newLogger())
 	assert.NilError(t, err, "failed to call GetModulesForService()")
 	assert.Equal(t, len(mods), 1, "expected exactly one module to be returned")
 	assert.Equal(t, mods[0].Version, "v0.6.0-rc.4", "expected module to match")
@@ -159,8 +168,47 @@ func TestCanUseBranch(t *testing.T) {
 				Channel: "main",
 			},
 		},
-	})
+	}, newLogger())
 	assert.NilError(t, err, "failed to call GetModulesForService()")
 	assert.Equal(t, len(mods), 1, "expected exactly one module to be returned")
 	assert.Equal(t, mods[0].Version, "main", "expected module to match")
 }
+
+func TestCanRespectChannels(t *testing.T) {
+	ctx := context.Background()
+	mods, err := modules.GetModulesForService(ctx, "", &configuration.ServiceManifest{
+		Name: "testing-service",
+		Modules: []*configuration.TemplateRepository{
+			{
+				Name:    "github.com/getoutreach/stencil-base",
+				Channel: "rc",
+			},
+			{
+				Name: "github.com/getoutreach/stencil-base",
+			},
+		},
+	}, newLogger())
+	assert.NilError(t, err, "failed to call GetModulesForService()")
+	assert.Equal(t, len(mods), 1, "expected exactly one module to be returned")
+	if !strings.Contains(mods[0].Version, "-rc.") {
+		t.Fatalf("expected module to be an RC, but got %s", mods[0].Version)
+	}
+}
+
+func TestShouldErrorOnTwoDifferentChannels(t *testing.T) {
+	ctx := context.Background()
+	_, err := modules.GetModulesForService(ctx, "", &configuration.ServiceManifest{
+		Name: "testing-service",
+		Modules: []*configuration.TemplateRepository{
+			{
+				Name:    "github.com/getoutreach/stencil-base",
+				Channel: "rc",
+			},
+			{
+				Name:    "github.com/getoutreach/stencil-base",
+				Channel: "unstable",
+			},
+		},
+	}, newLogger())
+	assert.ErrorContains(t, err, "previously resolved with channel", "expected GetModulesForService() to error")
+}

internal/modules/modules.go

@@ -17,6 +17,7 @@ import (
 	"github.com/getoutreach/gobox/pkg/cli/updater/resolver"
 	"github.com/getoutreach/stencil/pkg/configuration"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // resolvedModule is used to keep track of a module during the resolution
@@ -59,7 +60,8 @@ type resolution struct {
 
 // GetModulesForService returns a list of modules that have been resolved from the provided
 // service manifest, respecting constraints and channels as needed.
-func GetModulesForService(ctx context.Context, token cfg.SecretData, sm *configuration.ServiceManifest) ([]*Module, error) {
+func GetModulesForService(ctx context.Context, token cfg.SecretData,
+	sm *configuration.ServiceManifest, log logrus.FieldLogger) ([]*Module, error) {
 	// start resolving the top-level modules
 	modulesToResolve := make([]resolveModule, len(sm.Modules))
 	for i := range sm.Modules {
@@ -93,6 +95,10 @@ func GetModulesForService(ctx context.Context, token cfg.SecretData, sm *configu
 			channel:      resolv.conf.Channel,
 			parentModule: resolv.parent,
 		})
+		log.WithFields(logrus.Fields{
+			"module": importPath,
+			"parent": resolv.parent,
+		}).Debug("resolving module")
 
 		uri := "https://" + importPath
 		var version *resolver.Version
@@ -138,14 +144,19 @@ func GetModulesForService(ctx context.Context, token cfg.SecretData, sm *configu
 		for i := range mf.Modules {
 			modulesToResolve = append(modulesToResolve, resolveModule{
 				conf:   mf.Modules[i],
-				parent: importPath,
+				parent: importPath + "@" + version.String(),
 			})
 		}
 
 		// set the module on our resolved module
 		rm.Module = m
 		rm.version = version
 
+		log.WithFields(logrus.Fields{
+			"module":  importPath,
+			"version": version.GitRef(),
+		}).Debug("resolved module")
+
 		// resolve the next module
 		modulesToResolve = modulesToResolve[1:]
 	}
@@ -168,6 +179,23 @@ func getLatestModuleForConstraints(ctx context.Context, uri string, token cfg.Se
 		}
 	}
 
+	channel := m.conf.Channel
+	for _, r := range resolved[m.conf.Name].history {
+		// if we don't have a channel, or the channel is stable, check to see if
+		// the channel we last resolved with doesn't match the current channel requested.
+		//
+		// If it doesn't match, we don't know how to resolve the module, so we error.
+		if channel != "" && channel != resolver.StableChannel && r.channel != channel {
+			return nil, fmt.Errorf("unable to resolve module %s: "+
+				"module was previously resolved with channel %s (parent: %s), but now requires channel %s",
+				m.conf.Name, r.channel, r.parentModule, channel)
+		}
+
+		// use the first history entry that has a channel since we can't have multiple channels
+		channel = r.channel
+		break //nolint:staticcheck // Why: see above comment
+	}
+
 	// If the last version we resolved is mutable, it's impossible for us
 	// to compare the two, so we have to use it.
 	if rm, ok := resolved[m.conf.Name]; ok {
@@ -180,7 +208,7 @@ func getLatestModuleForConstraints(ctx context.Context, uri string, token cfg.Se
 
 	v, err := resolver.Resolve(ctx, token, &resolver.Criteria{
 		URL:           uri,
-		Channel:       m.conf.Channel,
+		Channel:       channel,
 		Constraints:   constraints,
 		AllowBranches: true,
 	})