Skip to content

Commit 4b3c568

Browse files
vinceprivincepri
committed
⚠️ Avoid shallow copies of webhooks and CRDs in testenv
Signed-off-by: Vince Prignano <vincepri@vmware.com>
1 parent 8e1263d commit 4b3c568

File tree

6 files changed

+50
-50
lines changed

6 files changed

+50
-50
lines changed

‎pkg/envtest/crd.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ type CRDInstallOptions struct {
6060
Paths []string
6161

6262
// CRDs is a list of CRDs to install
63-
CRDs []apiextensionsv1.CustomResourceDefinition
63+
CRDs []*apiextensionsv1.CustomResourceDefinition
6464

6565
// ErrorIfPathMissing will cause an error if a Path does not exist
6666
ErrorIfPathMissing bool
@@ -88,7 +88,7 @@ const defaultPollInterval = 100 * time.Millisecond
8888
const defaultMaxWait = 10 * time.Second
8989

9090
// InstallCRDs installs a collection of CRDs into a cluster by reading the crd yaml files from a directory.
91-
func InstallCRDs(config *rest.Config, options CRDInstallOptions) ([]apiextensionsv1.CustomResourceDefinition, error) {
91+
func InstallCRDs(config *rest.Config, options CRDInstallOptions) ([]*apiextensionsv1.CustomResourceDefinition, error) {
9292
defaultCRDOptions(&options)
9393

9494
// Read the CRD yamls into options.CRDs
@@ -140,7 +140,7 @@ func defaultCRDOptions(o *CRDInstallOptions) {
140140
}
141141

142142
// WaitForCRDs waits for the CRDs to appear in discovery.
143-
func WaitForCRDs(config *rest.Config, crds []apiextensionsv1.CustomResourceDefinition, options CRDInstallOptions) error {
143+
func WaitForCRDs(config *rest.Config, crds []*apiextensionsv1.CustomResourceDefinition, options CRDInstallOptions) error {
144144
// Add each CRD to a map of GroupVersion to Resource
145145
waitingFor := map[schema.GroupVersion]*sets.String{}
146146
for _, crd := range crds {
@@ -229,7 +229,7 @@ func UninstallCRDs(config *rest.Config, options CRDInstallOptions) error {
229229
for _, crd := range options.CRDs {
230230
crd := crd
231231
log.V(1).Info("uninstalling CRD", "crd", crd.GetName())
232-
if err := cs.Delete(context.TODO(), &crd); err != nil {
232+
if err := cs.Delete(context.TODO(), crd); err != nil {
233233
// If CRD is not found, we can consider success
234234
if !apierrors.IsNotFound(err) {
235235
return err
@@ -241,7 +241,7 @@ func UninstallCRDs(config *rest.Config, options CRDInstallOptions) error {
241241
}
242242

243243
// CreateCRDs creates the CRDs.
244-
func CreateCRDs(config *rest.Config, crds []apiextensionsv1.CustomResourceDefinition) error {
244+
func CreateCRDs(config *rest.Config, crds []*apiextensionsv1.CustomResourceDefinition) error {
245245
cs, err := client.New(config, client.Options{})
246246
if err != nil {
247247
return fmt.Errorf("unable to create client: %w", err)
@@ -255,7 +255,7 @@ func CreateCRDs(config *rest.Config, crds []apiextensionsv1.CustomResourceDefini
255255
err := cs.Get(context.TODO(), client.ObjectKey{Name: crd.GetName()}, existingCrd)
256256
switch {
257257
case apierrors.IsNotFound(err):
258-
if err := cs.Create(context.TODO(), &crd); err != nil {
258+
if err := cs.Create(context.TODO(), crd); err != nil {
259259
return fmt.Errorf("unable to create CRD %q: %w", crd.GetName(), err)
260260
}
261261
case err != nil:
@@ -267,7 +267,7 @@ func CreateCRDs(config *rest.Config, crds []apiextensionsv1.CustomResourceDefini
267267
return err
268268
}
269269
crd.SetResourceVersion(existingCrd.GetResourceVersion())
270-
return cs.Update(context.TODO(), &crd)
270+
return cs.Update(context.TODO(), crd)
271271
}); err != nil {
272272
return err
273273
}
@@ -277,7 +277,7 @@ func CreateCRDs(config *rest.Config, crds []apiextensionsv1.CustomResourceDefini
277277
}
278278

279279
// renderCRDs iterate through options.Paths and extract all CRD files.
280-
func renderCRDs(options *CRDInstallOptions) ([]apiextensionsv1.CustomResourceDefinition, error) {
280+
func renderCRDs(options *CRDInstallOptions) ([]*apiextensionsv1.CustomResourceDefinition, error) {
281281
var (
282282
err error
283283
info os.FileInfo
@@ -289,7 +289,7 @@ func renderCRDs(options *CRDInstallOptions) ([]apiextensionsv1.CustomResourceDef
289289
Name string
290290
}
291291

292-
crds := map[GVKN]apiextensionsv1.CustomResourceDefinition{}
292+
crds := map[GVKN]*apiextensionsv1.CustomResourceDefinition{}
293293

294294
for _, path := range options.Paths {
295295
var filePath = path
@@ -326,7 +326,7 @@ func renderCRDs(options *CRDInstallOptions) ([]apiextensionsv1.CustomResourceDef
326326
}
327327

328328
// Converting map to a list to return
329-
res := []apiextensionsv1.CustomResourceDefinition{}
329+
res := []*apiextensionsv1.CustomResourceDefinition{}
330330
for _, obj := range crds {
331331
res = append(res, obj)
332332
}
@@ -335,7 +335,7 @@ func renderCRDs(options *CRDInstallOptions) ([]apiextensionsv1.CustomResourceDef
335335

336336
// modifyConversionWebhooks takes all the registered CustomResourceDefinitions and applies modifications
337337
// to conditionally enable webhooks if the type is registered within the scheme.
338-
func modifyConversionWebhooks(crds []apiextensionsv1.CustomResourceDefinition, scheme *runtime.Scheme, webhookOptions WebhookInstallOptions) error {
338+
func modifyConversionWebhooks(crds []*apiextensionsv1.CustomResourceDefinition, scheme *runtime.Scheme, webhookOptions WebhookInstallOptions) error {
339339
if len(webhookOptions.LocalServingCAData) == 0 {
340340
return nil
341341
}
@@ -389,8 +389,8 @@ func modifyConversionWebhooks(crds []apiextensionsv1.CustomResourceDefinition, s
389389
}
390390

391391
// readCRDs reads the CRDs from files and Unmarshals them into structs.
392-
func readCRDs(basePath string, files []os.FileInfo) ([]apiextensionsv1.CustomResourceDefinition, error) {
393-
var crds []apiextensionsv1.CustomResourceDefinition
392+
func readCRDs(basePath string, files []os.FileInfo) ([]*apiextensionsv1.CustomResourceDefinition, error) {
393+
var crds []*apiextensionsv1.CustomResourceDefinition
394394

395395
// White list the file extensions that may contain CRDs
396396
crdExts := sets.NewString(".json", ".yaml", ".yml")
@@ -416,7 +416,7 @@ func readCRDs(basePath string, files []os.FileInfo) ([]apiextensionsv1.CustomRes
416416
if crd.Kind != "CustomResourceDefinition" || crd.Spec.Names.Kind == "" || crd.Spec.Group == "" {
417417
continue
418418
}
419-
crds = append(crds, *crd)
419+
crds = append(crds, crd)
420420
}
421421

422422
log.V(1).Info("read CRDs from file", "file", file.Name())

‎pkg/envtest/envtest_suite_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func initializeWebhookInEnvironment() {
5454
webhookPathV1 := "/failing"
5555

5656
env.WebhookInstallOptions = WebhookInstallOptions{
57-
ValidatingWebhooks: []admissionv1.ValidatingWebhookConfiguration{
57+
ValidatingWebhooks: []*admissionv1.ValidatingWebhookConfiguration{
5858
{
5959
ObjectMeta: metav1.ObjectMeta{
6060
Name: "deployment-validation-webhook-config",

‎pkg/envtest/envtest_test.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import (
3333
)
3434

3535
var _ = Describe("Test", func() {
36-
var crds []apiextensionsv1.CustomResourceDefinition
36+
var crds []*apiextensionsv1.CustomResourceDefinition
3737
var err error
3838
var s *runtime.Scheme
3939
var c client.Client
@@ -45,7 +45,7 @@ var _ = Describe("Test", func() {
4545

4646
// Initialize the client
4747
BeforeEach(func() {
48-
crds = []apiextensionsv1.CustomResourceDefinition{}
48+
crds = []*apiextensionsv1.CustomResourceDefinition{}
4949
s = scheme.Scheme
5050
err = apiextensionsv1.AddToScheme(s)
5151
Expect(err).NotTo(HaveOccurred())
@@ -69,7 +69,7 @@ var _ = Describe("Test", func() {
6969
continue
7070
}
7171
Expect(err).NotTo(HaveOccurred())
72-
Expect(c.Delete(context.TODO(), &crd)).To(Succeed())
72+
Expect(c.Delete(context.TODO(), crd)).To(Succeed())
7373
Eventually(func() bool {
7474
err := c.Get(context.TODO(), crdObjectKey, &placeholder)
7575
return apierrors.IsNotFound(err)
@@ -91,7 +91,7 @@ var _ = Describe("Test", func() {
9191
Expect(err).NotTo(HaveOccurred())
9292
Expect(crd.Spec.Names.Kind).To(Equal("Frigate"))
9393

94-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
94+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
9595
{
9696
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
9797
Group: "ship.example.com",
@@ -149,7 +149,7 @@ var _ = Describe("Test", func() {
149149
Expect(err).NotTo(HaveOccurred())
150150
Expect(crd.Spec.Names.Kind).To(Equal("Driver"))
151151

152-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
152+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
153153
{
154154
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
155155
Group: "bar.example.com",
@@ -256,7 +256,7 @@ var _ = Describe("Test", func() {
256256
Expect(err).NotTo(HaveOccurred())
257257
Expect(crd.Spec.Names.Kind).To(Equal("Config"))
258258

259-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
259+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
260260
{
261261
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
262262
Group: "foo.example.com",
@@ -305,7 +305,7 @@ var _ = Describe("Test", func() {
305305
Expect(err).NotTo(HaveOccurred())
306306
Expect(crd.Spec.Names.Kind).To(Equal("Foo"))
307307

308-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
308+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
309309
{
310310
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
311311
Group: "bar.example.com",
@@ -353,7 +353,7 @@ var _ = Describe("Test", func() {
353353
It("should return an error if the resource group version isn't found", func() {
354354
// Wait for a CRD where the Group and Version don't exist
355355
err := WaitForCRDs(env.Config,
356-
[]apiextensionsv1.CustomResourceDefinition{
356+
[]*apiextensionsv1.CustomResourceDefinition{
357357
{
358358
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
359359
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
@@ -383,7 +383,7 @@ var _ = Describe("Test", func() {
383383
Expect(err).NotTo(HaveOccurred())
384384

385385
// Wait for a CRD that doesn't exist, but the Group and Version do
386-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
386+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
387387
{
388388
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
389389
Group: "qux.example.com",
@@ -457,7 +457,7 @@ var _ = Describe("Test", func() {
457457
Expect(err).NotTo(HaveOccurred())
458458
Expect(crd.Spec.Names.Kind).To(Equal("Driver"))
459459

460-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
460+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
461461
{
462462
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
463463
Group: "bar.example.com",
@@ -586,7 +586,7 @@ var _ = Describe("Test", func() {
586586
Expect(err).NotTo(HaveOccurred())
587587
Expect(crd.Spec.Names.Kind).To(Equal("Driver"))
588588

589-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
589+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
590590
{
591591
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
592592
Group: "bar.example.com",
@@ -702,7 +702,7 @@ var _ = Describe("Test", func() {
702702
// Store resource version for comparison later on
703703
firstRV := crd.ResourceVersion
704704

705-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
705+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
706706
{
707707
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
708708
Group: "crew.example.com",
@@ -742,7 +742,7 @@ var _ = Describe("Test", func() {
742742
Expect(len(crd.Spec.Versions)).To(BeEquivalentTo(3))
743743
Expect(crd.ResourceVersion).NotTo(BeEquivalentTo(firstRV))
744744

745-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
745+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
746746
{
747747
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
748748
Group: "crew.example.com",
@@ -808,7 +808,7 @@ var _ = Describe("Test", func() {
808808
Expect(err).NotTo(HaveOccurred())
809809
Expect(crd.Spec.Names.Kind).To(Equal("Driver"))
810810

811-
err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
811+
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
812812
{
813813
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
814814
Group: "bar.example.com",

‎pkg/envtest/helper.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,18 @@ func mergePaths(s1, s2 []string) []string {
5151

5252
// mergeCRDs merges two CRD slices using their names.
5353
// This function makes no guarantees about order of the merged slice.
54-
func mergeCRDs(s1, s2 []apiextensionsv1.CustomResourceDefinition) []apiextensionsv1.CustomResourceDefinition {
55-
m := make(map[string]apiextensionsv1.CustomResourceDefinition)
54+
func mergeCRDs(s1, s2 []*apiextensionsv1.CustomResourceDefinition) []*apiextensionsv1.CustomResourceDefinition {
55+
m := make(map[string]*apiextensionsv1.CustomResourceDefinition)
5656
for _, obj := range s1 {
5757
m[obj.GetName()] = obj
5858
}
5959
for _, obj := range s2 {
6060
m[obj.GetName()] = obj
6161
}
62-
merged := make([]apiextensionsv1.CustomResourceDefinition, len(m))
62+
merged := make([]*apiextensionsv1.CustomResourceDefinition, len(m))
6363
i := 0
6464
for _, obj := range m {
65-
merged[i] = obj
65+
merged[i] = obj.DeepCopy()
6666
i++
6767
}
6868
return merged

‎pkg/envtest/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ type Environment struct {
136136
// CRDs is a list of CRDs to install.
137137
// If both this field and CRDs field in CRDInstallOptions are specified, the
138138
// values are merged.
139-
CRDs []apiextensionsv1.CustomResourceDefinition
139+
CRDs []*apiextensionsv1.CustomResourceDefinition
140140

141141
// CRDDirectoryPaths is a list of paths containing CRD yaml or json configs.
142142
// If both this field and Paths field in CRDInstallOptions are specified, the

‎pkg/envtest/webhook.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ type WebhookInstallOptions struct {
4545
Paths []string
4646

4747
// MutatingWebhooks is a list of MutatingWebhookConfigurations to install
48-
MutatingWebhooks []admissionv1.MutatingWebhookConfiguration
48+
MutatingWebhooks []*admissionv1.MutatingWebhookConfiguration
4949

5050
// ValidatingWebhooks is a list of ValidatingWebhookConfigurations to install
51-
ValidatingWebhooks []admissionv1.ValidatingWebhookConfiguration
51+
ValidatingWebhooks []*admissionv1.ValidatingWebhookConfiguration
5252

5353
// IgnoreErrorIfPathMissing will ignore an error if a DirectoryPath does not exist when set to true
5454
IgnoreErrorIfPathMissing bool
@@ -171,14 +171,14 @@ func (o *WebhookInstallOptions) Cleanup() error {
171171

172172
// WaitForWebhooks waits for the Webhooks to be available through API server.
173173
func WaitForWebhooks(config *rest.Config,
174-
mutatingWebhooks []admissionv1.MutatingWebhookConfiguration,
175-
validatingWebhooks []admissionv1.ValidatingWebhookConfiguration,
174+
mutatingWebhooks []*admissionv1.MutatingWebhookConfiguration,
175+
validatingWebhooks []*admissionv1.ValidatingWebhookConfiguration,
176176
options WebhookInstallOptions) error {
177177
waitingFor := map[schema.GroupVersionKind]*sets.String{}
178178

179179
for _, hook := range mutatingWebhooks {
180180
h := hook
181-
gvk, err := apiutil.GVKForObject(&h, scheme.Scheme)
181+
gvk, err := apiutil.GVKForObject(h, scheme.Scheme)
182182
if err != nil {
183183
return fmt.Errorf("unable to get gvk for MutatingWebhookConfiguration %s: %v", hook.GetName(), err)
184184
}
@@ -191,7 +191,7 @@ func WaitForWebhooks(config *rest.Config,
191191

192192
for _, hook := range validatingWebhooks {
193193
h := hook
194-
gvk, err := apiutil.GVKForObject(&h, scheme.Scheme)
194+
gvk, err := apiutil.GVKForObject(h, scheme.Scheme)
195195
if err != nil {
196196
return fmt.Errorf("unable to get gvk for ValidatingWebhookConfiguration %s: %v", hook.GetName(), err)
197197
}
@@ -288,7 +288,7 @@ func (o *WebhookInstallOptions) setupCA() error {
288288
return err
289289
}
290290

291-
func createWebhooks(config *rest.Config, mutHooks []admissionv1.MutatingWebhookConfiguration, valHooks []admissionv1.ValidatingWebhookConfiguration) error {
291+
func createWebhooks(config *rest.Config, mutHooks []*admissionv1.MutatingWebhookConfiguration, valHooks []*admissionv1.ValidatingWebhookConfiguration) error {
292292
cs, err := client.New(config, client.Options{})
293293
if err != nil {
294294
return err
@@ -298,14 +298,14 @@ func createWebhooks(config *rest.Config, mutHooks []admissionv1.MutatingWebhookC
298298
for _, hook := range mutHooks {
299299
hook := hook
300300
log.V(1).Info("installing mutating webhook", "webhook", hook.GetName())
301-
if err := ensureCreated(cs, &hook); err != nil {
301+
if err := ensureCreated(cs, hook); err != nil {
302302
return err
303303
}
304304
}
305305
for _, hook := range valHooks {
306306
hook := hook
307307
log.V(1).Info("installing validating webhook", "webhook", hook.GetName())
308-
if err := ensureCreated(cs, &hook); err != nil {
308+
if err := ensureCreated(cs, hook); err != nil {
309309
return err
310310
}
311311
}
@@ -357,7 +357,7 @@ func parseWebhook(options *WebhookInstallOptions) error {
357357

358358
// readWebhooks reads the Webhooks from files and Unmarshals them into structs
359359
// returns slice of mutating and validating webhook configurations.
360-
func readWebhooks(path string) ([]admissionv1.MutatingWebhookConfiguration, []admissionv1.ValidatingWebhookConfiguration, error) {
360+
func readWebhooks(path string) ([]*admissionv1.MutatingWebhookConfiguration, []*admissionv1.ValidatingWebhookConfiguration, error) {
361361
// Get the webhook files
362362
var files []os.FileInfo
363363
var err error
@@ -375,8 +375,8 @@ func readWebhooks(path string) ([]admissionv1.MutatingWebhookConfiguration, []ad
375375
// file extensions that may contain Webhooks
376376
resourceExtensions := sets.NewString(".json", ".yaml", ".yml")
377377

378-
var mutHooks []admissionv1.MutatingWebhookConfiguration
379-
var valHooks []admissionv1.ValidatingWebhookConfiguration
378+
var mutHooks []*admissionv1.MutatingWebhookConfiguration
379+
var valHooks []*admissionv1.ValidatingWebhookConfiguration
380380
for _, file := range files {
381381
// Only parse allowlisted file types
382382
if !resourceExtensions.Has(filepath.Ext(file.Name())) {
@@ -403,17 +403,17 @@ func readWebhooks(path string) ([]admissionv1.MutatingWebhookConfiguration, []ad
403403
if generic.APIVersion != admissionregv1 {
404404
return nil, nil, fmt.Errorf("only v1 is supported right now for MutatingWebhookConfiguration (name: %s)", generic.Name)
405405
}
406-
hook := admissionv1.MutatingWebhookConfiguration{}
407-
if err := yaml.Unmarshal(doc, &hook); err != nil {
406+
hook := &admissionv1.MutatingWebhookConfiguration{}
407+
if err := yaml.Unmarshal(doc, hook); err != nil {
408408
return nil, nil, err
409409
}
410410
mutHooks = append(mutHooks, hook)
411411
case generic.Kind == "ValidatingWebhookConfiguration":
412412
if generic.APIVersion != admissionregv1 {
413413
return nil, nil, fmt.Errorf("only v1 is supported right now for ValidatingWebhookConfiguration (name: %s)", generic.Name)
414414
}
415-
hook := admissionv1.ValidatingWebhookConfiguration{}
416-
if err := yaml.Unmarshal(doc, &hook); err != nil {
415+
hook := &admissionv1.ValidatingWebhookConfiguration{}
416+
if err := yaml.Unmarshal(doc, hook); err != nil {
417417
return nil, nil, err
418418
}
419419
valHooks = append(valHooks, hook)

0 commit comments

Comments
 (0)