feat(plugin): Add trivy-aws as a plugin

[simar7]

To be merged after aquasecurity/trivy-aws#153 gets merged.

[knqyf263]

Is it aws-scan, not aws?

[simar7]

Is it aws-scan, not aws?

@knqyf263 Yes we had no other choice as the Trivy AWS subcommand already is aws.

[knqyf263]

I thought old Trivy users would keep using the built-in trivy aws, and new users would install the plugin and also use the plugin command trivy aws. What is the advantage of changing the command name?

[simar7]

I thought old Trivy users would keep using the built-in trivy aws, and new users would install the plugin and also use the plugin command trivy aws. What is the advantage of changing the command name?

If an old Trivy user tries to install this plugin, Trivy picks up the subcommand over the plugin as that old version of Trivy will have the aws subcommand. It probably isn't what most people would do but still it is possible if a user doesn't want to upgrade Trivy for whatever reason but wants to use the new aws plugin functionality.

Another reason is that in this PR we will deprecate the aws subcommand but still have an information message for the user to learn how to install the plugin. If this user tries to install the plugin and it is named just aws, the same case will occur as above.

So for those reasons, I chose the plugin to have a completely different name. Not ideal to name it that but I didn't have a better idea.

We can still change it though to something else if you like or if you have a better idea.

[itaysk]

IIUC the implication is that the trivy aws plugin will only work from after we deprecate the built in target. if that's the worry i don't think it's that bad. also, if we really want to support this use case we can publish the plugin under another name, for the transition period. just my opinion

[knqyf263]

It probably isn't what most people would do but still it is possible if a user doesn't want to upgrade Trivy for whatever reason but wants to use the new aws plugin functionality.

Yes, it makes sense. However, trivy aws is currently bundled with Trivy and users need to update the Trivy version to use the new features of AWS scanning. Therefore, IMHO, it is not that bad to encourage existing users to update their Trivy version to use the new plugin (old Trivy users cannot use the new plugin).

Another reason is that in aquasecurity/trivy#6819 PR we will deprecate the aws subcommand but still have an information message for the user to learn how to install the plugin. If this user tries to install the plugin and it is named just aws, the same case will occur as above.

If that is a technical constraint, it could be achieved by looking at whether the aws subcommand is available.

diff --git a/pkg/commands/app.go b/pkg/commands/app.go
index 7746e1b70..f93475e28 100644
--- a/pkg/commands/app.go
+++ b/pkg/commands/app.go
@@ -97,7 +97,6 @@ func NewApp() *cobra.Command {
                NewKubernetesCommand(globalFlags),
                NewSBOMCommand(globalFlags),
                NewVersionCommand(globalFlags),
-               NewAWSCommand(globalFlags),
                NewVMCommand(globalFlags),
        )

@@ -109,6 +108,15 @@ func NewApp() *cobra.Command {
                rootCmd.AddCommand(plugins...)
        }

+       // TODO(backward-compatibility): Delete the subcommand after a while.
+       if cmd, _, _ := rootCmd.Find([]string{"aws"}); cmd == cmd.Root() { // "trivy aws" not installed
+               rootCmd.AddCommand(&cobra.Command{
+                       Hidden: true,
+                       Long:   "Trivy AWS is now available as an optional plugin. See github.com/aquasecurity/trivy-aws for details.",
+                       Use:    "aws",
+               })
+       }
+
        return rootCmd
 }

If you decide that using a different name is better in terms of user experience, I'm fine with that.

[knqyf263]

As @itaysk said, if we have a transition period where users can use both the built-in aws and the plugin, we definitely need another name. But according to aquasecurity/trivy#6819, we'll drop trivy aws support in the next version, right?

[knqyf263]

As @itaysk said, if we have a transition period where users can use both the built-in aws and the plugin, we definitely need another name. But according to aquasecurity/trivy#6819, we'll drop trivy aws support in the next version, right?

I don't think it's bad as trivy aws is experimental now. If we immediately delete the built-in subcommand, I just thought the same command name would provide a better user experience. But that's not a strong opinion, as I believe you guys have thought more deeply about user experiences!

[simar7]

As @itaysk said, if we have a transition period where users can use both the built-in aws and the plugin, we definitely need another name. But according to aquasecurity/trivy#6819, we'll drop trivy aws support in the next version, right?

I don't think it's bad as trivy aws is experimental now. If we immediately delete the built-in subcommand, I just thought the same command name would provide a better user experience. But that's not a strong opinion, as I believe you guys have thought more deeply about user experiences!

Actually I also thought the same but maybe took an overly cautious approach 😄 - I agree since it is experimental we can remove it immediately in next release and as a result have a clean trivy aws plugin to invoke. I'll make the necessary changes, thanks for the discussion!