Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

packer: log plugins used and their version/path #12567

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

packer: log plugins used and their version/path #12567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions bundled_plugin_versions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package main

import (
_ "embed"
"fmt"
"os"
"regexp"
"strings"

"github.com/hashicorp/packer/packer"
"golang.org/x/mod/modfile"
)

//go:embed go.mod
Copy link
Contributor

@nywilken nywilken Aug 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice to see how simple this turned out to be but it feels like a short lived change given that their versions haven't changed in the past 4 releases and that we will be removing bundle plugins soon. But I do think logging the versions for external plugins makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principle I agree with you, so that leaves us with a couple alternatives:

  1. We don't show bundled plugin usage: this is relatively straightforward, as we can remove this code. What I don't like is that it doesn't reflect the true plugin usage (though we do see the warning for bundled plugins which may lead us on which plugin is used, but isn't perfect, especially for JSON).
  2. We track bundled plugins, without the version info: doing so means that we need to infer which plugins are loaded, and track their version as bundled. This may not result in code that is as separated as this one though, as we'll probably need to hack something elsewhere in the codebase.
  3. This solution.

Given those alternatives, I know I would prefer option 3 since the work is already done, the code is straightforward, and concentrated (besides the call to this in main), so when we remove bundled plugins, this will be equally easy to cleanup (at least compared to solution 2).
Let me know what you think on that, I would have liked to include this in 1.9.3, but we can push it to a later version if there are reservations on this implementation.

var mod string

var pluginRegex = regexp.MustCompile("packer-plugin-.*$")

func GetBundledPluginVersions() map[string]packer.PluginSpec {
pluginSpecs := map[string]packer.PluginSpec{}

mods, err := modfile.Parse("", []byte(mod), nil)
if err != nil {
panic(fmt.Sprintf("failed to parse embedded modfile: %s", err))
}

for _, req := range mods.Require {
if pluginRegex.MatchString(req.Mod.Path) {
pluginName := pluginRegex.FindString(req.Mod.Path)
pluginShortName := strings.Replace(pluginName, "packer-plugin-", "", 1)
pluginSpecs[pluginShortName] = packer.PluginSpec{
Name: pluginShortName,
Version: fmt.Sprintf("bundled (%s)", req.Mod.Version),
Path: os.Args[0],
}
}
}

return pluginSpecs
}
2 changes: 2 additions & 0 deletions command/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ func (c *BuildCommand) RunContext(buildCtx context.Context, cla *BuildArgs) int
return ret
}

c.LogPluginUsage(packerStarter)

hcpRegistry, diags := registry.New(packerStarter, c.Ui)
ret = writeDiags(c.Ui, nil, diags)
if ret != 0 {
Expand Down
113 changes: 111 additions & 2 deletions command/meta.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import (
"flag"
"fmt"
"io"
"log"
"os"
"sort"
"strings"

"github.com/hashicorp/hcl/v2"
Expand Down Expand Up @@ -257,7 +259,9 @@ func (m *Meta) detectBundledPluginsJSON(core *packer.Core) []string {
}
}

return compileBundledPluginList(bundledPlugins)
bundledPluginList := compileBundledPluginList(bundledPlugins)

return bundledPluginList
}

var knownPluginPrefixes = map[string]string{
Expand Down Expand Up @@ -354,5 +358,110 @@ func (m *Meta) detectBundledPluginsHCL2(config *hcl2template.PackerConfig) []str
}
}

return compileBundledPluginList(bundledPlugins)
bundledPluginList := compileBundledPluginList(bundledPlugins)

return bundledPluginList
}

func (m *Meta) LogPluginUsage(handler packer.Handler) {
switch h := handler.(type) {
case *packer.Core:
m.logPluginUsageJSON(h)
case *hcl2template.PackerConfig:
m.logPluginUsageHCL2(handler.(*hcl2template.PackerConfig))
}
}

func (m *Meta) logPluginUsageJSON(c *packer.Core) {
usedPlugins := map[string]packer.PluginSpec{}

tmpl := c.Template
if tmpl == nil {
panic("No template parsed. This is a Packer bug which should be reported, please open an issue on the project's issue tracker.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a pattern but I think throwing a panic for failures that are not critical to the execution of Packer can be more a blocker to the user than helpful. Especially, if there are unknown cases that cause an expected failure. For example the panics happening in the tests runs.

If we can't get this information for logging or displaying a warning to a user we should log the failure and not stop the build.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you, but at this time, if the template isn't properly parsed, this means that we've changed something in the logic, and we should be made aware ASAP, not later when we investigate why we don't see a warning/error anymore, and we notice this log.
At least that's my reasoning, and why I think some sanity-check panics like these are still relevant.
That being said, I'm not against the idea of changing this to a log/UI.Error, let me know which makes sense in your opinion.

}

for _, b := range tmpl.Builders {
// Check since internal components are not registered as components
ps, ok := m.CoreConfig.Components.PluginConfig.GetSpecForPlugin(b.Type)
if ok {
usedPlugins[ps.Name] = ps
}
}

for _, p := range tmpl.Provisioners {
// Check since internal components are not registered as components
ps, ok := m.CoreConfig.Components.PluginConfig.GetSpecForPlugin(p.Type)
if ok {
usedPlugins[ps.Name] = ps
}
}

for _, pps := range tmpl.PostProcessors {
for _, pp := range pps {
// Check since internal components are not registered as components
ps, ok := m.CoreConfig.Components.PluginConfig.GetSpecForPlugin(pp.Type)
if ok {
usedPlugins[ps.Name] = ps
}
}
}

logPlugins(usedPlugins)
}

func (m *Meta) logPluginUsageHCL2(config *hcl2template.PackerConfig) {
usedPlugins := map[string]packer.PluginSpec{}

for _, b := range config.Builds {
for _, src := range b.Sources {
ps, ok := m.CoreConfig.Components.PluginConfig.GetSpecForPlugin(src.Type)
if ok {
usedPlugins[ps.Name] = ps
}
}

for _, p := range b.ProvisionerBlocks {
ps, ok := m.CoreConfig.Components.PluginConfig.GetSpecForPlugin(p.PType)
if ok {
usedPlugins[ps.Name] = ps
}
}

for _, pps := range b.PostProcessorsLists {
for _, pp := range pps {
ps, ok := m.CoreConfig.Components.PluginConfig.GetSpecForPlugin(pp.PType)
if ok {
usedPlugins[ps.Name] = ps
}
}
}
}

for _, ds := range config.Datasources {
ps, ok := m.CoreConfig.Components.PluginConfig.GetSpecForPlugin(ds.Type)
if ok {
usedPlugins[ps.Name] = ps
}
}

logPlugins(usedPlugins)
}

func logPlugins(usedPlugins map[string]packer.PluginSpec) {
// Could happen if no plugin is loaded and we only rely on internal components
if len(usedPlugins) == 0 {
return
}

pluginNames := make([]string, 0, len(usedPlugins))
for pn := range usedPlugins {
pluginNames = append(pluginNames, pn)
}
sort.Strings(pluginNames)

log.Printf("[INFO] - Used plugins")
for _, pn := range pluginNames {
ps := usedPlugins[pn]
log.Printf(fmt.Sprintf("*\t%s - %s: %s", ps.Name, ps.Version, ps.Path))
}
}
2 changes: 2 additions & 0 deletions command/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ func (c *ValidateCommand) RunContext(ctx context.Context, cla *ValidateArgs) int
return ret
}

c.LogPluginUsage(packerStarter)

_, diags = packerStarter.GetBuilds(packer.GetBuildsOptions{
Only: cla.Only,
Except: cla.Except,
Expand Down
7 changes: 6 additions & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,6 @@ func loadConfig() (*config, error) {
PluginMinPort: 10000,
PluginMaxPort: 25000,
KnownPluginFolders: packer.PluginFolders("."),

// BuilderRedirects
BuilderRedirects: map[string]string{

Expand Down Expand Up @@ -391,7 +390,13 @@ func loadConfig() (*config, error) {
//"vsphere": "github.com/hashicorp/vsphere",
//"vsphere-template": "github.com/hashicorp/vsphere",
},
PluginComponents: map[string]packer.PluginSpec{},
}
bundledPlugins := GetBundledPluginVersions()
for pn, ps := range bundledPlugins {
config.Plugins.PluginComponents[pn] = ps
}

if err := config.Plugins.Discover(); err != nil {
return nil, err
}
Expand Down
70 changes: 70 additions & 0 deletions packer/plugin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,20 @@ var defaultChecksummer = plugingetter.Checksummer{
Hash: sha256.New(),
}

type PluginSpec struct {
Name string
Path string
Version string
}

func (ps PluginSpec) String() string {
return fmt.Sprintf(`
Plugin: %s
Path: %s
Version: %s`,
ps.Name, ps.Path, ps.Version)
}

// PluginConfig helps load and use packer plugins
type PluginConfig struct {
KnownPluginFolders []string
Expand All @@ -51,6 +65,50 @@ type PluginConfig struct {
DatasourceRedirects map[string]string
ProvisionerRedirects map[string]string
PostProcessorRedirects map[string]string
PluginComponents map[string]PluginSpec
}

// GetSpecForComponent returns a pluginspec if the component requested exists in the loaded plugin specs
//
// If it does not, the returned boolean will be false.
func (c PluginConfig) GetSpecForPlugin(name string) (PluginSpec, bool) {
names := getPartsFromComponent(name)

for _, name := range names {
pc, ok := c.PluginComponents[name]
if ok {
return pc, true
}
}

return PluginSpec{}, false
}

// getPartsFromComponent splits the plugin on '-' and returns a list of potential names
// starting from the longest, and ending on the shortest.
//
// Ex:
// ```go
//
// getPartsFromComponent("plugin-name-component-name") => [
// "plugin-name-component-name",
// "plugin-name-component",
// "plugin-name",
// "plugin",
// ]
//
// ```
func getPartsFromComponent(name string) []string {
rets := []string{}

parts := strings.Split(name, "-")

for len(parts) > 0 {
rets = append(rets, strings.Join(parts, "-"))
parts = parts[:len(parts)-1]
}

return rets
}

// PACKERSPACE is used to represent the spaces that separate args for a command
Expand Down Expand Up @@ -280,6 +338,12 @@ func (c *PluginConfig) DiscoverMultiPlugin(pluginName, pluginPath string) error
return err
}

pluginSpec := PluginSpec{
Name: pluginName,
Path: pluginPath,
Version: desc.Version,
}

pluginPrefix := pluginName + "-"

for _, builderName := range desc.Builders {
Expand Down Expand Up @@ -340,6 +404,12 @@ func (c *PluginConfig) DiscoverMultiPlugin(pluginName, pluginPath string) error
log.Printf("found external %v datasource from %s plugin", desc.Datasources, pluginName)
}

if c.PluginComponents == nil {
c.PluginComponents = map[string]PluginSpec{}
}

c.PluginComponents[pluginName] = pluginSpec

return nil
}

Expand Down