Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: filter package components with strategy interface #2321

Merged
merged 51 commits into from Mar 21, 2024
Merged

Conversation

Noxsios
Copy link
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 <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Copy link

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

Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
@Noxsios Noxsios marked this pull request as ready for review February 21, 2024 20:35
Noxsios and others added 4 commits February 21, 2024 15:10
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Copy link
Member

@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.

src/pkg/packager/dev.go Outdated Show resolved Hide resolved
src/pkg/packager/filters/deploy.go Outdated Show resolved Hide resolved
src/pkg/packager/filters/deploy_test.go Outdated Show resolved Hide resolved
src/pkg/packager/filters/deploy_test.go Outdated Show resolved Hide resolved
src/pkg/packager/filters/deploy_test.go Outdated Show resolved Hide resolved
src/pkg/packager/filters/deploy_test.go Outdated Show resolved Hide resolved
src/pkg/packager/filters/deploy_test.go Show resolved Hide resolved
src/pkg/packager/filters/deploy_test.go Outdated Show resolved Hide resolved
src/pkg/packager/filters/deploy.go Outdated Show resolved Hide resolved
src/types/component.go Outdated Show resolved Hide resolved
src/types/component.go Outdated Show resolved Hide resolved
Noxsios and others added 3 commits March 4, 2024 10:51
Co-authored-by: Lucas Rodriguez <lucas.rodriguez9616@gmail.com>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Copy link
Contributor

@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

src/pkg/interactive/prompt.go Show resolved Hide resolved
src/pkg/packager/deploy.go Outdated Show resolved Hide resolved
src/pkg/packager/sources/oci.go Outdated Show resolved Hide resolved
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
AustinAbro321
AustinAbro321 previously approved these changes Mar 15, 2024
Copy link
Contributor

@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 <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
lucasrod16
lucasrod16 previously approved these changes Mar 18, 2024
src/pkg/packager/deploy.go Outdated Show resolved Hide resolved
src/pkg/packager/mirror.go Show resolved Hide resolved
src/pkg/packager/mirror.go Outdated Show resolved Hide resolved
src/pkg/packager/sources/tarball.go Outdated Show resolved Hide resolved
src/pkg/packager/sources/oci.go Outdated Show resolved Hide resolved
src/pkg/packager/sources/oci.go Outdated Show resolved Hide resolved
Signed-off-by: razzle <harry@razzle.cloud>
Signed-off-by: razzle <harry@razzle.cloud>
@Noxsios Noxsios requested a review from Racer159 March 19, 2024 21:11
AustinAbro321
AustinAbro321 previously approved these changes Mar 20, 2024
Signed-off-by: razzle <harry@razzle.cloud>
src/pkg/packager/deploy.go Show resolved Hide resolved
@Noxsios Noxsios merged commit 95c42ff into main Mar 21, 2024
30 checks passed
@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
Projects
No open projects
Status: Approved
Development

Successfully merging this pull request may close these issues.

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