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.

Sign up for GitHub

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

Jump to bottom

⚠ Avoid shallow copies of webhooks and CRDs in testenv #1667

Merged
merged 1 commit into from Sep 21, 2021
Merged

⚠ Avoid shallow copies of webhooks and CRDs in testenv #1667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/builder/builder_suite_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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