From c146f9d0e0d7aa2089eb58c9533d276a762a9b97 Mon Sep 17 00:00:00 2001 From: Jared Allard Date: Tue, 13 Sep 2022 16:52:22 -0700 Subject: [PATCH] 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 --- bootstrap.lock | 2 +- cmd/stencil/stencil.go | 5 +++ internal/cmd/stencil/stencil.go | 2 +- internal/modules/module_test.go | 64 ++++++++++++++++++++++++++++----- internal/modules/modules.go | 34 ++++++++++++++++-- 5 files changed, 94 insertions(+), 13 deletions(-) diff --git a/bootstrap.lock b/bootstrap.lock index 0f369a20..2b7465fe 100644 --- a/bootstrap.lock +++ b/bootstrap.lock @@ -3,4 +3,4 @@ version: v10.3.0-rc.11 generated: 2022-08-10T14:54:43Z versions: - devbase: main + devbase: v2.4.0-rc.3 diff --git a/cmd/stencil/stencil.go b/cmd/stencil/stencil.go index 68f26bf2..f9b2cf3a 100644 --- a/cmd/stencil/stencil.go +++ b/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{ diff --git a/internal/cmd/stencil/stencil.go b/internal/cmd/stencil/stencil.go index 8c2314c1..ad750afb 100644 --- a/internal/cmd/stencil/stencil.go +++ b/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") } diff --git a/internal/modules/module_test.go b/internal/modules/module_test.go index 181dda5c..648cb1c7 100644 --- a/internal/modules/module_test.go +++ b/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") +} diff --git a/internal/modules/modules.go b/internal/modules/modules.go index a3034762..c89d7b30 100644 --- a/internal/modules/modules.go +++ b/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,7 +144,7 @@ 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(), }) } @@ -146,6 +152,11 @@ func GetModulesForService(ctx context.Context, token cfg.SecretData, sm *configu 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, })