Skip to content

feat!: filter package components with strategy interface#2321

Merged
Noxsios merged 51 commits intomainfrom
filter-strategy
Mar 21, 2024
Merged

feat!: filter package components with strategy interface#2321
Noxsios merged 51 commits intomainfrom
filter-strategy

Conversation

@Noxsios
Copy link
Copy Markdown
Contributor

@Noxsios Noxsios commented Feb 21, 2024

Description

Consolidate component filtering logic into a filters package. Each filter is an implementation of

// ComponentFilterStrategy is a strategy interface for filtering components.
type ComponentFilterStrategy interface {
	Apply(types.ZarfPackage) ([]types.ZarfComponent, error)
}

Public construction functions return instances of this interface not instances of their underlying structs. Consumers should be fully cognizant of which filter they are using, and should not be making a common wrapper function (eg. NewFilter(args...) to dynamically return a filter.

ex:

func Empty() ComponentFilterStrategy {
	return &emptyFilter{}
}

// emptyFilter is a filter that does nothing.
type emptyFilter struct{}

// Apply returns the components unchanged.
func (f *emptyFilter) Apply(pkg types.ZarfPackage) ([]types.ZarfComponent, error) {
	return pkg.Components, nil
}

BREAKING CHANGES: This changes the interface signatures on sources.PackageSource to reflect the new behavior whereupon the zarf.yaml is loaded into memory within the sources load operations. This allows for filtering to take place during load and for the zarf.yaml in memory to reflect these filter operations.

Related Issue

Fixes #2320

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

Signed-off-by: razzle <[email protected]>
Signed-off-by: razzle <[email protected]>
Signed-off-by: razzle <[email protected]>
Signed-off-by: razzle <[email protected]>
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 21, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit eed0129
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/65fb699ce378b700080003cf

@Noxsios Noxsios marked this pull request as ready for review February 21, 2024 20:35
Copy link
Copy Markdown
Contributor

@lucasrod16 lucasrod16 left a comment

Choose a reason for hiding this comment

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

I don't have a strong opinion on whether filter implementations return the strategy interface or the implementation struct.

My preference would be to return the concrete implementation struct because it makes the code a bit easier to navigate imo. For example, when I click into Apply() in vs code in the client code, it takes me to the interface method signature instead of the implementation. It would be nice to be taken directly to the implementation, but not a big enough reason to outweigh other factors.

Comment thread src/pkg/packager/dev.go Outdated
Comment thread src/pkg/packager/filters/deploy.go Outdated
Comment thread src/pkg/packager/filters/deploy_test.go Outdated
Comment thread src/pkg/packager/filters/deploy_test.go Outdated
Comment thread src/pkg/packager/filters/deploy_test.go Outdated
Comment thread src/pkg/packager/filters/deploy_test.go Outdated
Comment thread src/pkg/packager/filters/deploy_test.go
Comment thread src/pkg/packager/filters/deploy_test.go Outdated
Comment thread src/pkg/packager/filters/deploy.go Outdated
Comment thread src/types/component.go Outdated
Comment thread src/types/component.go Outdated
Noxsios added 4 commits March 13, 2024 18:57
Signed-off-by: razzle <[email protected]>
Signed-off-by: razzle <[email protected]>
Signed-off-by: razzle <[email protected]>
Signed-off-by: razzle <[email protected]>
Copy link
Copy Markdown
Member

@AustinAbro321 AustinAbro321 left a comment

Choose a reason for hiding this comment

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

A few small things mostly questions on my end

Comment thread src/pkg/interactive/prompt.go
Comment thread src/pkg/packager/deploy.go Outdated
Comment thread src/pkg/packager/sources/oci.go Outdated
AustinAbro321
AustinAbro321 previously approved these changes Mar 15, 2024
Copy link
Copy Markdown
Member

@AustinAbro321 AustinAbro321 left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: razzle <[email protected]>
lucasrod16
lucasrod16 previously approved these changes Mar 18, 2024
Comment thread src/pkg/packager/deploy.go Outdated
Comment thread src/pkg/packager/mirror.go
Comment thread src/pkg/packager/mirror.go Outdated
Comment thread src/pkg/packager/sources/tarball.go Outdated
Comment thread src/pkg/packager/sources/oci.go Outdated
Comment thread src/pkg/packager/sources/oci.go Outdated
@Noxsios Noxsios requested a review from Racer159 March 19, 2024 21:11
AustinAbro321
AustinAbro321 previously approved these changes Mar 20, 2024
Comment thread src/pkg/packager/deploy.go
@Noxsios Noxsios merged commit 95c42ff into main Mar 21, 2024
@Noxsios Noxsios deleted the filter-strategy branch March 21, 2024 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OCI deploy with default: true component fails to find image blob

4 participants