Skip to content

Commit 139cc3c

Browse files
authored
Merge pull request kubernetes#127254 from liggitt/test-filter
Fix unit tests for filtering
2 parents 386a6af + 6a41706 commit 139cc3c

File tree

2 files changed

+29
-4
lines changed

2 files changed

+29
-4
lines changed

staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/filter_test.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ func TestFilter(t *testing.T) {
529529
*simpleLabelSelector,
530530
},
531531
}),
532-
enableSelectors: true,
532+
enableSelectors: false,
533533
compatibilityVersion: v130,
534534
},
535535
{
@@ -602,7 +602,7 @@ func TestFilter(t *testing.T) {
602602
attributes: newValidAttribute(&podObject, false),
603603
results: []EvaluationResult{
604604
{
605-
EvalResult: celtypes.True,
605+
Error: fmt.Errorf("fieldSelector"),
606606
},
607607
},
608608
authorizer: newAuthzAllowMatch(authorizer.AttributesRecord{
@@ -615,6 +615,7 @@ func TestFilter(t *testing.T) {
615615
Verb: "create",
616616
APIVersion: "*",
617617
}),
618+
enableSelectors: false,
618619
compatibilityVersion: v131,
619620
},
620621
{
@@ -871,6 +872,7 @@ func TestFilter(t *testing.T) {
871872

872873
for _, tc := range cases {
873874
t.Run(tc.name, func(t *testing.T) {
875+
environment.DisableBaseEnvSetCachingForTests()
874876
if tc.enableSelectors {
875877
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true)
876878
}
@@ -896,6 +898,7 @@ func TestFilter(t *testing.T) {
896898
if f == nil {
897899
t.Fatalf("unexpected nil validator")
898900
}
901+
899902
validations := tc.validations
900903
CompilationResults := f.(*filter).compilationResults
901904
require.Equal(t, len(validations), len(CompilationResults))
@@ -913,8 +916,13 @@ func TestFilter(t *testing.T) {
913916
}
914917
require.Equal(t, len(evalResults), len(tc.results))
915918
for i, result := range tc.results {
919+
if result.Error != nil && evalResults[i].Error == nil {
920+
t.Errorf("Expected error result containing '%v' but got non-error", result.Error)
921+
continue
922+
}
916923
if result.Error != nil && !strings.Contains(evalResults[i].Error.Error(), result.Error.Error()) {
917924
t.Errorf("Expected result '%v' but got '%v'", result.Error, evalResults[i].Error)
925+
continue
918926
}
919927
if result.Error == nil && evalResults[i].Error != nil {
920928
t.Errorf("Expected result '%v' but got error '%v'", result.EvalResult, evalResults[i].Error)

staging/src/k8s.io/apiserver/pkg/cel/environment/base.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,19 @@ var StrictCostOpt = VersionedOptions{
191191
},
192192
}
193193

194+
// cacheBaseEnvs controls whether calls to MustBaseEnvSet are cached.
195+
// Defaults to true, may be disabled by calling DisableBaseEnvSetCachingForTests.
196+
var cacheBaseEnvs = true
197+
198+
// DisableBaseEnvSetCachingForTests clears and disables base env caching.
199+
// This is only intended for unit tests exercising MustBaseEnvSet directly with different enablement options.
200+
// It does not clear other initialization paths that may cache results of calling MustBaseEnvSet.
201+
func DisableBaseEnvSetCachingForTests() {
202+
cacheBaseEnvs = false
203+
baseEnvs.Clear()
204+
baseEnvsWithOption.Clear()
205+
}
206+
194207
// MustBaseEnvSet returns the common CEL base environments for Kubernetes for Version, or panics
195208
// if the version is nil, or does not have major and minor components.
196209
//
@@ -216,7 +229,9 @@ func MustBaseEnvSet(ver *version.Version, strictCost bool) *EnvSet {
216229
}
217230
entry, _, _ = baseEnvsSingleflight.Do(key, func() (interface{}, error) {
218231
entry := mustNewEnvSet(ver, baseOpts)
219-
baseEnvs.Store(key, entry)
232+
if cacheBaseEnvs {
233+
baseEnvs.Store(key, entry)
234+
}
220235
return entry, nil
221236
})
222237
} else {
@@ -225,7 +240,9 @@ func MustBaseEnvSet(ver *version.Version, strictCost bool) *EnvSet {
225240
}
226241
entry, _, _ = baseEnvsWithOptionSingleflight.Do(key, func() (interface{}, error) {
227242
entry := mustNewEnvSet(ver, baseOptsWithoutStrictCost)
228-
baseEnvsWithOption.Store(key, entry)
243+
if cacheBaseEnvs {
244+
baseEnvsWithOption.Store(key, entry)
245+
}
229246
return entry, nil
230247
})
231248
}

0 commit comments

Comments
 (0)