Skip to content

Commit

Permalink
⚠️ Avoid shallow copies of webhooks and CRDs in testenv
Browse files Browse the repository at this point in the history 
Signed-off-by: Vince Prignano <vincepri@vmware.com>
  • Loading branch information
@vincepri
vincepri committed Sep 20, 2021
1 parent 8e1263d commit a14a68c
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 52 deletions.
2 changes: 1 addition & 1 deletion pkg/builder/builder_suite_test.go
View file
Expand Up @@ -78,7 +78,7 @@ var _ = AfterSuite(func() {
func addCRDToEnvironment(env *envtest.Environment, gvks ...schema.GroupVersionKind) {
for _, gvk := range gvks {
plural, singular := meta.UnsafeGuessKindToResource(gvk)
crd := apiextensionsv1.CustomResourceDefinition{
crd := &apiextensionsv1.CustomResourceDefinition{
TypeMeta: metav1.TypeMeta{
APIVersion: "apiextensions.k8s.io/v1",
Kind: "CustomResourceDefinition",
Expand Down
28 changes: 14 additions & 14 deletions pkg/envtest/crd.go
View file
Expand Up @@ -60,7 +60,7 @@ type CRDInstallOptions struct {
Paths []string

// CRDs is a list of CRDs to install
CRDs []apiextensionsv1.CustomResourceDefinition
CRDs []*apiextensionsv1.CustomResourceDefinition

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

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

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

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

// CreateCRDs creates the CRDs.
func CreateCRDs(config *rest.Config, crds []apiextensionsv1.CustomResourceDefinition) error {
func CreateCRDs(config *rest.Config, crds []*apiextensionsv1.CustomResourceDefinition) error {
cs, err := client.New(config, client.Options{})
if err != nil {
return fmt.Errorf("unable to create client: %w", err)
Expand All @@ -255,7 +255,7 @@ func CreateCRDs(config *rest.Config, crds []apiextensionsv1.CustomResourceDefini
err := cs.Get(context.TODO(), client.ObjectKey{Name: crd.GetName()}, existingCrd)
switch {
case apierrors.IsNotFound(err):
if err := cs.Create(context.TODO(), &crd); err != nil {
if err := cs.Create(context.TODO(), crd); err != nil {
return fmt.Errorf("unable to create CRD %q: %w", crd.GetName(), err)
}
case err != nil:
Expand All @@ -267,7 +267,7 @@ func CreateCRDs(config *rest.Config, crds []apiextensionsv1.CustomResourceDefini
return err
}
crd.SetResourceVersion(existingCrd.GetResourceVersion())
return cs.Update(context.TODO(), &crd)
return cs.Update(context.TODO(), crd)
}); err != nil {
return err
}
Expand All @@ -277,7 +277,7 @@ func CreateCRDs(config *rest.Config, crds []apiextensionsv1.CustomResourceDefini
}

// renderCRDs iterate through options.Paths and extract all CRD files.
func renderCRDs(options *CRDInstallOptions) ([]apiextensionsv1.CustomResourceDefinition, error) {
func renderCRDs(options *CRDInstallOptions) ([]*apiextensionsv1.CustomResourceDefinition, error) {
var (
err error
info os.FileInfo
Expand All @@ -289,7 +289,7 @@ func renderCRDs(options *CRDInstallOptions) ([]apiextensionsv1.CustomResourceDef
Name string
}

crds := map[GVKN]apiextensionsv1.CustomResourceDefinition{}
crds := map[GVKN]*apiextensionsv1.CustomResourceDefinition{}

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

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

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

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

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

log.V(1).Info("read CRDs from file", "file", file.Name())
Expand Down
2 changes: 1 addition & 1 deletion pkg/envtest/envtest_suite_test.go
View file
Expand Up @@ -54,7 +54,7 @@ func initializeWebhookInEnvironment() {
webhookPathV1 := "/failing"

env.WebhookInstallOptions = WebhookInstallOptions{
ValidatingWebhooks: []admissionv1.ValidatingWebhookConfiguration{
ValidatingWebhooks: []*admissionv1.ValidatingWebhookConfiguration{
{
ObjectMeta: metav1.ObjectMeta{
Name: "deployment-validation-webhook-config",
Expand Down
28 changes: 14 additions & 14 deletions pkg/envtest/envtest_test.go
View file
Expand Up @@ -33,7 +33,7 @@ import (
)

var _ = Describe("Test", func() {
var crds []apiextensionsv1.CustomResourceDefinition
var crds []*apiextensionsv1.CustomResourceDefinition
var err error
var s *runtime.Scheme
var c client.Client
Expand All @@ -45,7 +45,7 @@ var _ = Describe("Test", func() {

// Initialize the client
BeforeEach(func() {
crds = []apiextensionsv1.CustomResourceDefinition{}
crds = []*apiextensionsv1.CustomResourceDefinition{}
s = scheme.Scheme
err = apiextensionsv1.AddToScheme(s)
Expect(err).NotTo(HaveOccurred())
Expand All @@ -69,7 +69,7 @@ var _ = Describe("Test", func() {
continue
}
Expect(err).NotTo(HaveOccurred())
Expect(c.Delete(context.TODO(), &crd)).To(Succeed())
Expect(c.Delete(context.TODO(), crd)).To(Succeed())
Eventually(func() bool {
err := c.Get(context.TODO(), crdObjectKey, &placeholder)
return apierrors.IsNotFound(err)
Expand All @@ -91,7 +91,7 @@ var _ = Describe("Test", func() {
Expect(err).NotTo(HaveOccurred())
Expect(crd.Spec.Names.Kind).To(Equal("Frigate"))

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

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

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

err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Group: "bar.example.com",
Expand Down Expand Up @@ -353,7 +353,7 @@ var _ = Describe("Test", func() {
It("should return an error if the resource group version isn't found", func() {
// Wait for a CRD where the Group and Version don't exist
err := WaitForCRDs(env.Config,
[]apiextensionsv1.CustomResourceDefinition{
[]*apiextensionsv1.CustomResourceDefinition{
{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Versions: []apiextensionsv1.CustomResourceDefinitionVersion{
Expand Down Expand Up @@ -383,7 +383,7 @@ var _ = Describe("Test", func() {
Expect(err).NotTo(HaveOccurred())

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

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

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

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

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

err = WaitForCRDs(env.Config, []apiextensionsv1.CustomResourceDefinition{
err = WaitForCRDs(env.Config, []*apiextensionsv1.CustomResourceDefinition{
{
Spec: apiextensionsv1.CustomResourceDefinitionSpec{
Group: "bar.example.com",
Expand Down
8 changes: 4 additions & 4 deletions pkg/envtest/helper.go
View file
Expand Up @@ -51,18 +51,18 @@ func mergePaths(s1, s2 []string) []string {

// mergeCRDs merges two CRD slices using their names.
// This function makes no guarantees about order of the merged slice.
func mergeCRDs(s1, s2 []apiextensionsv1.CustomResourceDefinition) []apiextensionsv1.CustomResourceDefinition {
m := make(map[string]apiextensionsv1.CustomResourceDefinition)
func mergeCRDs(s1, s2 []*apiextensionsv1.CustomResourceDefinition) []*apiextensionsv1.CustomResourceDefinition {
m := make(map[string]*apiextensionsv1.CustomResourceDefinition)
for _, obj := range s1 {
m[obj.GetName()] = obj
}
for _, obj := range s2 {
m[obj.GetName()] = obj
}
merged := make([]apiextensionsv1.CustomResourceDefinition, len(m))
merged := make([]*apiextensionsv1.CustomResourceDefinition, len(m))
i := 0
for _, obj := range m {
merged[i] = obj
merged[i] = obj.DeepCopy()
i++
}
return merged
Expand Down
2 changes: 1 addition & 1 deletion pkg/envtest/server.go
View file
Expand Up @@ -136,7 +136,7 @@ type Environment struct {
// CRDs is a list of CRDs to install.
// If both this field and CRDs field in CRDInstallOptions are specified, the
// values are merged.
CRDs []apiextensionsv1.CustomResourceDefinition
CRDs []*apiextensionsv1.CustomResourceDefinition

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

0 comments on commit a14a68c

Please sign in to comment.