wip: fix: deduplication of artifact and dependency references #869

c2ndev · 2025-12-22T16:12:13Z

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area cli

What this PR does / why we need it:
This PR fixes an incorrect behavior during the artifact installation phase.
During this step, falcoctl attempts to deduplicate artifact and dependency references to avoid multiple pulls from the container registry.
However, this deduplication currently does not work correctly.

Which issue(s) this PR fixes:

Fixes #868

Special notes for your reviewer:

…all command Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

poiana · 2025-12-22T16:12:23Z

Welcome @cannarelladev! It looks like this is your first PR to falcosecurity/falcoctl 🎉

leogr · 2025-12-22T16:15:19Z

/milestone v0.12.0

Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

Copilot

Pull request overview

This PR fixes the deduplication logic for artifact and dependency references during the installation phase. Previously, falcoctl attempted to deduplicate artifacts but the logic was incorrect, leading to multiple pulls from the container registry for the same artifact.

Key changes:

Refactored artifact preparation logic into a new prepareArtifactList function that properly deduplicates user-provided artifact references before dependency resolution
Modified ResolveDeps to accept separate resolvers for artifact configs and references, enabling proper resolution of dependency references

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
cmd/artifact/install/install.go	Extracted deduplication logic into new `prepareArtifactList` function; removed redundant `resolvedRef` variable usage throughout installation flow
cmd/artifact/install/deps.go	Added `refResolver` type; updated `ResolveDeps` signature to accept both config and ref resolvers; removed duplicate reference check for user inputs; added reference resolution for dependencies
cmd/artifact/install/deps_test.go	Updated test cases to use new `configResolver` naming and added `refResolver` to match new `ResolveDeps` signature
cmd/artifact/install/install_test.go	Removed trailing whitespace

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T10:16:33Z

cmd/artifact/install/install.go

+func (o *artifactInstallOptions) prepareArtifactList(ctx context.Context, puller *puller.Puller, args []string) (refs []string, err error) {
+	logger := o.Printer.Logger
+
+	resolver := refResolver(func(ref string) (string, error) {
+		return o.IndexCache.ResolveReference(ref)
+	})
+
+	configResolver := artifactConfigResolver(func(ref string) (*oci.RegistryResult, error) {
+		artifactConfig, err := puller.ArtifactConfig(ctx, ref, o.platformOS, o.platformArch)
+		if err != nil {
+			return nil, err
+		}
+
+		return &oci.RegistryResult{
+			Config: *artifactConfig,
+		}, nil
+	})
+
+	seen := make(map[string]struct{}, len(args))
+	resolved := make([]string, 0, len(args))
+
+	// Compute unique resolved references
+	for _, arg := range args {
+		ref, err := resolver(arg)
+		if err != nil {
+			return nil, err
+		}
+
+		if _, ok := seen[ref]; ok {
+			continue
+		}
+		seen[ref] = struct{}{}
+		resolved = append(resolved, ref)
+	}


The new prepareArtifactList function implements the core deduplication logic that this PR is meant to fix. However, there are no tests that verify this deduplication behavior works correctly when duplicate artifact references are provided. Consider adding a test case that verifies duplicate artifacts (either exact duplicates or artifacts that resolve to the same reference) are properly deduplicated.

Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

leogr

Hey @cannarelladev

This is a great improvement! However, I found a couple of issues (see my comment below).

Furthermore, I have a doubt 🤔

This PR introduces a behavior change in how we handle an imperfect artifact's references list as input.
Previously, the user received a clear error when they supplied conflicting refs for the same artifact. Now, falcoctl will ignore duplicates, but the actual installed version will depend purely on argument order.

Since the intended goal here is to deduplicate, we might want to follow a predictable logic for which duplicate wins (e.g., "highest version wins" rather than “last argument wins”). Using the highest compatible version is a pattern we already used for alternative dependencies, and I guess it should work in this case as well.

leogr · 2025-12-23T11:12:50Z

cmd/artifact/install/install.go

-
-		if signatures[resolvedRef] == nil {
+		if signatures[ref] == nil {
 			if sig := o.IndexCache.SignatureForIndexRef(ref); sig != nil {


With this change, now we pass full references to SignatureForIndexRef(), which by design returns nil for full refs. As a result, the signature verification is effectively skipped every time, even when it exists in the index.

Now:

❯ ./falcoctl artifact install k8saudit k8saudit-rules 2025-12-23 11:29:45 INFO Resolving dependencies ... 2025-12-23 11:29:48 INFO Installing artifacts └ refs: [ghcr.io/falcosecurity/plugins/plugin/k8saudit:latest ghcr.io/falcosecurity/plugins/ruleset/k8saudit:latest ghcr.io/falcosecurity/plugins/plugin/json:0.7.0] 2025-12-23 11:29:48 INFO Preparing to pull artifact ref: ghcr.io/falcosecurity/plugins/plugin/k8saudit:latest 2025-12-23 11:29:48 INFO Pulling layer 78495c93d6f6 2025-12-23 11:29:49 INFO Pulling layer 163468d93bfc 2025-12-23 11:29:49 INFO Pulling layer 37afdf131d6f 2025-12-23 11:29:49 ERROR cannot use directory "/usr/share/falco/plugins" as install destination: /usr/share/falco/plugins is not writable

Before:

❯ falcoctl artifact install k8saudit k8saudit-rules 2025-12-23 12:01:39 INFO Resolving dependencies ... 2025-12-23 12:01:42 INFO Installing artifacts └ refs: [ghcr.io/falcosecurity/plugins/plugin/k8saudit:latest ghcr.io/falcosecurity/plugins/ruleset/k8saudit:latest json:0.7.0] 2025-12-23 12:01:42 INFO Preparing to pull artifact ref: ghcr.io/falcosecurity/plugins/plugin/k8saudit:latest 2025-12-23 12:01:43 INFO Pulling layer 78495c93d6f6 2025-12-23 12:01:43 INFO Pulling layer 163468d93bfc 2025-12-23 12:01:43 INFO Pulling layer 37afdf131d6f 2025-12-23 12:01:43 INFO Verifying signature for artifact └ digest: ghcr.io/falcosecurity/plugins/plugin/k8saudit@sha256:7310019ee3b7f89d503df7e50db15fbe771b4e003125782960fd0e8da43b0905 2025-12-23 12:01:44 INFO Signature successfully verified! 2025-12-23 12:01:44 ERROR cannot use directory "/usr/share/falco/plugins" as install destination: /usr/share/falco/plugins is not writable

N.B. Ignore the error, I just didn't use sudo. The point is that the Verifying signature for artifact step is no longer happening.

Further note, I can't recall why SignatureForIndexRef() can't work if we have a full reference. It may be worth investigating the reasons for that, or whether it could be improved to accept full references as well.

Fixed: the implementation now reuses an existing data structure to store all artifact names for SignatureForIndexRef().
Additionally, the behavior of falcoctl artifact install when duplicate references are provided is not defined anywhere in the repository. Previously, this resulted in an error; now, duplicates are handled by selecting the highest version.

leogr · 2025-12-23T13:05:10Z

cmd/artifact/install/deps.go

 					if alternativeVer.Compare(*existing.ver) > 0 {
 						if err := upsertMap(alternative.Name + ":" + alternativeVer.String()); err != nil {
 							return nil, err
 						}
 					}


Before this changeset, this function took a single artifactConfigResolver and assumed it resolved references by calling IndexCache.ResolveReference(ref). Now we need to explicitly call resolver before inserting it in the map.
This is correctly handled for primary deps (see line 173 below), but we are missing the resolver call here for alternative deps.

Indeed, if we try to install k8saudit-eks:0.2.1 together with k8saudit-rules (which, among alternatives, declares k8saudit-eks:0.4.0) we see the problem:

❯ ./falcoctl artifact install k8saudit-eks:0.2.1 k8saudit-rules 2025-12-23 14:01:21 INFO Resolving dependencies ... 2025-12-23 14:01:24 ERROR unable to create new repository with ref k8saudit-eks:0.4.0: invalid reference: missing registry or repository

Before, the alternative dependency was correctly resolved:

❯ falcoctl artifact install k8saudit-eks:0.2.1 k8saudit-rules 2025-12-23 14:01:08 INFO Resolving dependencies ... 2025-12-23 14:01:12 INFO Installing artifacts refs: [k8saudit-eks:0.4.0 ghcr.io/falcosecurity/plugins/ruleset/k8saudit:latest json:0.7.0] 2025-12-23 14:01:12 INFO Preparing to pull artifact ref: ghcr.io/falcosecurity/plugins/plugin/k8saudit-eks:0.4.0 ...

the decoupling between "resolver" and pull of "configResolver" is intentional, in fact the goal is to avoid that inside the ResolveDeps the inRefs will be resolved again (they are already resolved).
Anyway the described behavior about the alternative reference it's a bug. I'm fixing it resolving properly the ref before upserting the map.

…ve artifact reference, add tests to prepareArtifactList Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

poiana · 2026-01-07T15:35:37Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: c2ndev
Once this PR has been reviewed and has the lgtm label, please assign fededp for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

c2ndev added 2 commits December 22, 2025 16:23

fixed logic to deduplicate refs and dependencies during artifact inst…

4ad21ac

…all command Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

create specific method to prepare reference list

f0dff59

Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

github-project-automation bot added this to Falco Roadmap Dec 22, 2025

github-project-automation bot moved this to Todo in Falco Roadmap Dec 22, 2025

poiana added kind/bug dco-signoff: yes area/cli labels Dec 22, 2025

poiana requested review from cpanato and leogr December 22, 2025 16:12

poiana added the size/L label Dec 22, 2025

c2ndev changed the title ~~Fix: deduplication of artifact references and dependencies~~ Fix: deduplication of artifact and dependency references Dec 22, 2025

poiana added this to the v0.12.0 milestone Dec 22, 2025

c2ndev changed the title ~~Fix: deduplication of artifact and dependency references~~ wip: fix: deduplication of artifact and dependency references Dec 23, 2025

poiana added the do-not-merge/work-in-progress label Dec 23, 2025

fix: add cap to slice and map, make the PrepareArtifactList not exported

e76c598

Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

leogr requested a review from Copilot December 23, 2025 10:11

Copilot started reviewing on behalf of leogr December 23, 2025 10:11 View session

Copilot AI reviewed Dec 23, 2025

View reviewed changes

fix: change copyright year

6f66f3a

Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

leogr reviewed Dec 23, 2025

View reviewed changes

poiana added size/XXL and removed size/L labels Dec 23, 2025

fix: avoid usage of resolved refs during signature, resolve alternati…

8ce8d13

…ve artifact reference, add tests to prepareArtifactList Signed-off-by: cannarelladev <cannarella.dev@gmail.com>

c2ndev force-pushed the fix/artifact-ref-deps-dedup branch from 86e89b3 to 8ce8d13 Compare January 7, 2026 15:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

wip: fix: deduplication of artifact and dependency references #869

wip: fix: deduplication of artifact and dependency references #869

c2ndev commented Dec 22, 2025

Uh oh!

poiana commented Dec 22, 2025

Uh oh!

leogr commented Dec 22, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 23, 2025

Uh oh!

leogr left a comment

Uh oh!

leogr Dec 23, 2025 •

edited

Loading

Uh oh!

c2ndev Dec 23, 2025

Uh oh!

leogr Dec 23, 2025

Uh oh!

c2ndev Dec 23, 2025

Uh oh!

poiana commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wip: fix: deduplication of artifact and dependency references #869

Are you sure you want to change the base?

wip: fix: deduplication of artifact and dependency references #869

Conversation

c2ndev commented Dec 22, 2025

Uh oh!

poiana commented Dec 22, 2025

Uh oh!

leogr commented Dec 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

leogr left a comment

Choose a reason for hiding this comment

Uh oh!

leogr Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c2ndev Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

leogr Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

c2ndev Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

poiana commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

leogr Dec 23, 2025 •

edited

Loading