veertuinc / packer-plugin-veertu-anka

🚛 A packer plugin for Veertu's Anka Build Cloud & Anka Virtualization software which allows you to run single-use macOS VMs like you would with docker.
https://veertu.com
MIT License
70 stars 15 forks source link

Packer 1.11.0 and plugin versions #163

Closed lbajolet-hashicorp closed 5 months ago

lbajolet-hashicorp commented 7 months ago

Hi there!

I'm one of the Packer core developers, and I'm opening this issue to let you know of the upcoming changes to Packer, specifically with v1.11.0 (tentatively scheduled for an April release), namely we're changing how plugins are discovered and loaded. We're officialising pre-release support for our tooling, we've settled on suporting -dev versions only to keep the logic simple, any other pre-release (e.g. -alpha1, -beta1, etc.) will be rejected by Packer.

With this change, we're also being a bit more strict on the plugins we report and load. Namely we're mandating SHA256SUM files alongside the binary, and versions need to be coherent between the file name, and the version reported by describe.

This last point is one we noticed will be a problem for your plugin.

I attempted to install the latest version of the plugin earlier today, and noticed that they're published under 3.2.1-d39014ecca6c8d4480bf4e6f8c4bc35a27af3976. Our changes make it impossible for Packer to load the plugin, as we are enforcing semver more accurately, and are supporting either final release versions, or -dev prereleases only.

As it stands then, unfortunately the plugin versions you compile cannot be loaded with Packer v1.11.0. In order for it to be compatible, I suggest (if you want to keep the SHA of the commit from which the binary was built) making the SHA a metadata of the version, with a + instead of a -. This is not supported by the 1.11.0-alpha we released last Monday to be clear, but this is something we're working on supporting ASAP, as we discovered some plugins that embed metadata in their version, which none of our plugins does.

I suggest also updating your install target to leverage packer plugins install --path so it will work when 1.11.0 is released later this year.

Please let me know your thoughts about this, and if you need help with the code feel free to reach out, I can set time aside to assist.

Thanks!

PS: For information and testing, we have released a blog post recently to explain the changes coming up to Packer, and we've also released Packer v1.11.0-alpha with which you can test your plugin.

NorseGaud commented 7 months ago

Here is how we set the version:

package main
. . .
var (
    version = ""
    commit  = ""
)

func main() {
    pps := plugin.NewSet()
    pps.RegisterBuilder("vm-create", new(anka.Builder))
    pps.RegisterBuilder("vm-clone", new(anka.Builder))
    pps.RegisterPostProcessor("registry-push", new(ankaregistry.PostProcessor))
    var pluginVersion = packerVersion.InitializePluginVersion(version, commit)
    pps.SetVersion(pluginVersion)
    log.Printf("plugin version: %s", pluginVersion)
    err := pps.Run()
    if err != nil {
        fmt.Fprintln(os.Stderr, err.Error())
        os.Exit(1)
    }
}

Yet, the SDK itself is setting the wrong version with -:

func (p *PluginVersion) FormattedVersion() string {
    var versionString bytes.Buffer
    fmt.Fprintf(&versionString, "%s", p.version)
    if p.versionPrerelease != "" {
        fmt.Fprintf(&versionString, "-%s", p.versionPrerelease)
        . . .
❯ ./dist/packer-plugin-veertu-anka_v4.0.0_x5.0_darwin_arm64
2024/03/21 13:18:40 plugin version: 4.0.0-ba9d8973e7ca55bb66d89bdbfbd04b2f1651b306
needs at least one argument

@lbajolet-hashicorp , is this what you meant by This is not supported by the 1.11.0-alpha we released last Monday to be clear, but this is something we're working on supporting ASAP, as we discovered some plugins that embed metadata in their version, which none of our plugins does.?

Should I fork and modify the SDK to do + instead?

lbajolet-hashicorp commented 7 months ago

Hi @NorseGaud,

I see indeed that you're doing things a bit differently from our scaffolding, to be clear: nothing wrong with that, you're in charge of the plugin code :)

I would advise against changing things in the SDK, the InitializePluginVersion function expects a version and a pre-release, which is unset in your case, no problems with that, you can pass the empty string as preversion if you don't want it.

I suggest if you want to add the commit SHA as metadata for your version to maybe add it manually? SetVersion accepts virtually any string under the sun (note to myself: we should probably try to check/enforce this at compile time), so you could aff metadata with a fmt.Sprintf.

Here's my suggestion:

24          var pluginVersion = packerVersion.InitializePluginVersion(version, "")
25          pps.SetVersion(fmt.Sprintf("%s+%s", pluginVersion, commit))

If I'm not mistaken, you'll end-up with a version like this: 1.0.0+123abcdef, which is valid and will be supported by 1.11.0 when we release it.

Edit: nevermind what I just said, this is not the case, the SetVersion function requires a PluginVersion. This may need some SDK update, not to change how PreRelease works, but to maybe add an alternative for setting a raw plugin version, or an alternative with Metadata support. I'll try to come back to you with something.

FYI regarding metadata in the plugin's version: the alpha release from last week doesn't support it completely, we'll be releasing a new alpha version in the coming week to address that problem and make sure that there won't be any ambiguity when discovering/loading plugins.

Let me know what you think, does that approach look sound to you?

NorseGaud commented 7 months ago

Thanks @lbajolet-hashicorp , I'll definitely go with your suggestion to not set the commit right now. I think that's fine to get me unblocked and moving forward. Thanks again!!

lbajolet-hashicorp commented 7 months ago

That also works, thanks for the quick update @NorseGaud.

Apologies for the misunderstanding regarding SetVersion, I'll see to update the SDK soon so that metadata support is something that can be used, I don't like imposing changes that make you lose information. I can't promise this will be a thing by the time we release 1.11.0, but hopefully this will be available not too long after.

Thanks again!

NorseGaud commented 7 months ago

All good :)

NorseGaud commented 7 months ago

https://github.com/hashicorp/packer-plugin-sdk/pull/228