Not friendly "upgrade" message for detached plugins

[ahmetb]

After doing multi-index migration on my local index, I ran kubectl krew upgrade.

Updated the local copy of plugin index.
Upgrading plugin: access-matrix
Skipping plugin access-matrix, it is already on the newest version
WARNING: failed to upgrade plugin "detached/", skipping (error: open /Users/ahmetb/.krew/index/detached/plugins/cert-manager.yaml: no such file or directory)
WARNING: failed to upgrade plugin "detached/", skipping (error: open /Users/ahmetb/.krew/index/detached/plugins/chonk.yaml: no such file or directory)
Upgrading plugin: ctx
Skipping plugin ctx, it is already on the newest version
WARNING: failed to upgrade plugin "detached/", skipping (error: open /Users/ahmetb/.krew/index/detached/plugins/example.yaml: no such file or directory)
Upgrading plugin: fleet
Upgraded plugin: fleet
WARNING: You installed plugin "fleet" from the krew-index plugin repository.
   These plugins are not audited for security by the Krew maintainers.
   Run them at your own risk.
Upgrading plugin: get-all
Skipping plugin get-all, it is already on the newest version
Upgrading plugin: gs
^C

Specifically:

WARNING: failed to upgrade plugin "detached/", skipping (error: open /Users/ahmetb/.krew/index/detached/plugins/cert-manager.yaml: no such file or directory)

In this case, yes this plugin is indeed detached, but the error message says:

1. it's looking at a plugin named detached/
2. there should be a dir at $HOME/.krew/index/detached/

I forgot what the expected behavior should be while running upgrade on all plugins that include detached ones (skip or fail?) but regardless, the error message is less than ideal.

Would be good to fix before we tag v0.4. (Though, this only affects plugin developer persona.)

/cc @chriskim06
/cc @corneliusweig
/kind bug
/priority p1

[chriskim06]

I've been trying to look for a place where we might have discussed this before but I can't find anything. I feel like we decided to skip upgrade for detached plugins, but since I can't find that what do you guys think?

[ahmetb]

We show these plugins in the list as detached/PLUGIN_NAME. so I'm not entirely certain why the plugin name parses into "detached/" in the first place.

The answer probably comes down to "do we wanna handle index name detached specially in upgrade cmd".

Is this string coming from leaving the source index field empty in the receipt? Or do we actually write this down into the receipt?

[chriskim06]

We write that as the index in the receipt when a plugin is installed via manifest #568 (I'm trying to find the original comment where this spawned from came from here #555 (comment)). Normal krew-index plugins will have default written into their receipts once they're upgraded (and treated as default if there isn't an index in the receipt).

[corneliusweig]

If I understand correctly, the problem is that after index migration, there are plugins which should be considered detached, but have an empty index origin in their receipt, so that they are treated as default?

This sounds like we need to os.Stat() the index manifest, if we find a receipt with an empty source index to decide if it should be treated as detached or default.

---

I guess to avoid special cases in a number of places, we should fill the source index field in manifests during the index migration. This means that the index migration is no longer as simple as moving a folder, but it will enable us to just skip anything with a detached source during krew upgrade.

[chriskim06]

Ya so if a user installed a plugin with a manifest using 0.3.x then the index in the receipt will be empty and will be treated as default. That shouldn't really be an issue though I think since before 0.4.x the only index that existed was krew-index. From 0.4.x onward however, plugins that are installed with a manifest will be given the detached index.

I think it makes sense to skip detached plugins during upgrade (and have some leveled log explaining detached plugins are skipped) rather than trying to upgrade them and failing for those plugins.

[ahmetb]

I think this is not a release blocker (only impacts plugin developers).

If we detect index=="detached" and skip upgrading those (with a WARNING) would be good.

[chriskim06]

ok that's good, I'll still try to get this and the other cleanup issue taken care of soon