Skip to content

refactor: move util.Wildcard into its own package#2853

Merged
sozercan merged 2 commits into
open-policy-agent:masterfrom
xrstf:move-wildcard-type
Jul 5, 2023
Merged

refactor: move util.Wildcard into its own package#2853
sozercan merged 2 commits into
open-policy-agent:masterfrom
xrstf:move-wildcard-type

Conversation

@xrstf
Copy link
Copy Markdown
Contributor

@xrstf xrstf commented Jun 29, 2023

What this PR does / why we need it:
We ran into an issue when updating our product to controller-runtime 0.15. We tried gatekeeper v3.13.0-beta.1 and ended up with

github.com/open-policy-agent/gatekeeper/v3/pkg/util
# github.com/open-policy-agent/gatekeeper/v3/pkg/util
../../github.com/open-policy-agent/gatekeeper/pkg/util/pack.go:44:9: cannot use func(obj client.Object) []reconcile.Request {…} (value of type func(obj client.Object) []reconcile.Request) as handler.MapFunc value in return statement
../../github.com/open-policy-agent/gatekeeper/pkg/util/pack.go:70:9: cannot use func(obj client.Object) []reconcile.Request {…} (value of type func(obj client.Object) []reconcile.Request) as handler.MapFunc value in return statement
../../github.com/open-policy-agent/gatekeeper/pkg/util/pack.go:75:13: not enough arguments in call to mf
        have (*unstructured.Unstructured)
        want (context.Context, client.Object)

This is because the signature for MapFuncs has changed to include a context.Context now.

To solve this issue, this PR moves the util.Wildcard type into its own package (analogous to pkg/version). This removes the dependency from apis/config/v1alpha1 to pkg/util, thereby removing the need to compile-in a full-blown controller-runtime library for anyone who wants to use Gatekeeper's config/v1alpha1 API.

Signed-off-by: Christoph Mewes <christoph@kubermatic.com>
@xrstf xrstf force-pushed the move-wildcard-type branch from ddb3c76 to 60f73b8 Compare June 29, 2023 21:06
@xrstf xrstf changed the title move util.Wildcard into its own package refactor: move util.Wildcard into its own package Jun 29, 2023
Copy link
Copy Markdown
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR!

@maxsmythe maxsmythe requested review from ritazh and sozercan July 1, 2023 00:49
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 1, 2023

Codecov Report

❌ Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.48%. Comparing base (d8f2bbc) to head (8c1a529).
⚠️ Report is 811 commits behind head on master.

Files with missing lines Patch % Lines
pkg/controller/config/process/excluder.go 25.00% 3 Missing ⚠️
pkg/mutation/match/zz_generated.deepcopy.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2853      +/-   ##
==========================================
- Coverage   53.55%   53.48%   -0.07%     
==========================================
  Files         133      133              
  Lines       11536    11536              
==========================================
- Hits         6178     6170       -8     
- Misses       4882     4888       +6     
- Partials      476      478       +2     
Flag Coverage Δ
unittests 53.48% <16.66%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

5 participants