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.

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

e2e test for KafkaTopic webhook #1031

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion pkg/webhooks/kafkatopic_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func (s *KafkaTopicValidator) checkExistingKafkaTopicCRs(ctx context.Context,
}
}
if foundKafkaTopic != nil {
logMsg := fmt.Sprintf("kafkaTopic CR '%s' in namesapce '%s' is already referencing to Kafka topic '%s'",
logMsg := fmt.Sprintf("kafkaTopic CR '%s' in namespace '%s' is already referencing to Kafka topic '%s'",
foundKafkaTopic.Name, foundKafkaTopic.Namespace, foundKafkaTopic.Spec.Name)
return field.Invalid(field.NewPath("spec").Child("name"), foundKafkaTopic.Spec.Name, logMsg), nil
}
Expand Down
61 changes: 32 additions & 29 deletions tests/e2e/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ require (
github.com/twmb/franz-go v1.13.5
k8s.io/apiextensions-apiserver v0.26.4
k8s.io/apimachinery v0.26.4
k8s.io/utils v0.0.0-20230726121419-3b25d923346b
sigs.k8s.io/yaml v1.3.0
)

Expand All @@ -35,22 +36,49 @@ require (
github.com/eapache/go-resiliency v1.3.0 // indirect
github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21 // indirect
github.com/eapache/queue v1.1.0 // indirect
github.com/emicklei/go-restful/v3 v3.9.0 // indirect
github.com/evanphx/json-patch v5.6.0+incompatible // indirect
github.com/evanphx/json-patch/v5 v5.6.0 // indirect
github.com/fatih/color v1.13.0 // indirect
github.com/go-logr/zapr v1.2.3 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/hashicorp/go-uuid v1.0.3 // indirect
github.com/huandu/xstrings v1.3.3 // indirect
github.com/iancoleman/orderedmap v0.2.0 // indirect
github.com/jcmturner/aescts/v2 v2.0.0 // indirect
github.com/jcmturner/dnsutils/v2 v2.0.0 // indirect
github.com/jcmturner/gofork v1.7.6 // indirect
github.com/jcmturner/gokrb5/v8 v8.4.3 // indirect
github.com/jcmturner/rpc/v2 v2.0.3 // indirect
github.com/klauspost/compress v1.16.3 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.16 // indirect
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/onsi/ginkgo v1.16.5 // indirect
github.com/pavlo-v-chernykh/keystore-go/v4 v4.4.1 // indirect
github.com/pierrec/lz4/v4 v4.1.17 // indirect
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect
github.com/spf13/cast v1.4.1 // indirect
github.com/tidwall/gjson v1.9.3 // indirect
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.0 // indirect
github.com/twmb/franz-go/pkg/kmsg v1.4.0 // indirect
github.com/wayneashleyberry/terminal-dimensions v1.0.0 // indirect
go.uber.org/zap v1.24.0 // indirect
sigs.k8s.io/controller-runtime v0.14.6 // indirect
)

require (
github.com/emicklei/go-restful/v3 v3.9.0 // indirect
github.com/evanphx/json-patch v5.6.0+incompatible // indirect
github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32 // indirect
github.com/go-errors/errors v1.0.2-0.20180813162953-d98b870cc4e0 // indirect
github.com/go-logr/logr v1.2.4 // indirect
github.com/go-logr/zapr v1.2.3 // indirect
github.com/go-openapi/jsonpointer v0.19.5 // indirect
github.com/go-openapi/jsonreference v0.20.0 // indirect
github.com/go-openapi/swag v0.19.14 // indirect
github.com/go-sql-driver/mysql v1.4.1 // indirect
github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/google/gnostic v0.6.9 // indirect
github.com/google/go-cmp v0.5.9 // indirect
github.com/google/gofuzz v1.2.0 // indirect
Expand All @@ -59,50 +87,27 @@ require (
github.com/gruntwork-io/go-commons v0.8.0 // indirect
github.com/hashicorp/errwrap v1.1.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
github.com/hashicorp/go-uuid v1.0.3 // indirect
github.com/huandu/xstrings v1.3.3 // indirect
github.com/iancoleman/orderedmap v0.2.0 // indirect
github.com/imdario/mergo v0.3.13 // indirect
github.com/jcmturner/aescts/v2 v2.0.0 // indirect
github.com/jcmturner/dnsutils/v2 v2.0.0 // indirect
github.com/jcmturner/gofork v1.7.6 // indirect
github.com/jcmturner/gokrb5/v8 v8.4.3 // indirect
github.com/jcmturner/rpc/v2 v2.0.3 // indirect
github.com/jmespath/go-jmespath v0.4.0 // indirect
github.com/josharian/intern v1.0.0 // indirect
github.com/json-iterator/go v1.1.12 // indirect
github.com/klauspost/compress v1.16.3 // indirect
github.com/mailru/easyjson v0.7.6 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-isatty v0.0.16 // indirect
github.com/mattn/go-zglob v0.0.2-0.20190814121620-e3c945676326 // indirect
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/moby/spdystream v0.2.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/onsi/ginkgo v1.16.5 // indirect
github.com/pavlo-v-chernykh/keystore-go/v4 v4.4.1 // indirect
github.com/pierrec/lz4/v4 v4.1.17 // indirect
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/pquerna/otp v1.2.0 // indirect
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect
github.com/russross/blackfriday/v2 v2.1.0 // indirect
github.com/spf13/cast v1.4.1 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/stretchr/testify v1.8.1 // indirect
github.com/tidwall/gjson v1.9.3 // indirect
github.com/tidwall/match v1.1.1 // indirect
github.com/tidwall/pretty v1.2.0 // indirect
github.com/twmb/franz-go/pkg/kmsg v1.4.0 // indirect
github.com/urfave/cli v1.22.2 // indirect
github.com/wayneashleyberry/terminal-dimensions v1.0.0 // indirect
go.uber.org/atomic v1.9.0 // indirect
go.uber.org/multierr v1.6.0 // indirect
go.uber.org/zap v1.24.0 // indirect
golang.org/x/crypto v0.7.0 // indirect
golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 // indirect
golang.org/x/net v0.10.0 // indirect
Expand All @@ -121,8 +126,6 @@ require (
k8s.io/client-go v0.26.4 // indirect
k8s.io/klog/v2 v2.80.1 // indirect
k8s.io/kube-openapi v0.0.0-20221207184640-f3cff1453715 // indirect
k8s.io/utils v0.0.0-20221128185143-99ec85e7a448 // indirect
sigs.k8s.io/controller-runtime v0.14.6 // indirect
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect
)
4 changes: 2 additions & 2 deletions tests/e2e/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -845,8 +845,8 @@ k8s.io/klog/v2 v2.80.1 h1:atnLQ121W371wYYFawwYx1aEY2eUfs4l3J72wtgAwV4=
k8s.io/klog/v2 v2.80.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0=
k8s.io/kube-openapi v0.0.0-20221207184640-f3cff1453715 h1:tBEbstoM+K0FiBV5KGAKQ0kuvf54v/hwpldiJt69w1s=
k8s.io/kube-openapi v0.0.0-20221207184640-f3cff1453715/go.mod h1:+Axhij7bCpeqhklhUTe3xmOn6bWxolyZEeyaFpjGtl4=
k8s.io/utils v0.0.0-20221128185143-99ec85e7a448 h1:KTgPnR10d5zhztWptI952TNtt/4u5h3IzDXkdIMuo2Y=
k8s.io/utils v0.0.0-20221128185143-99ec85e7a448/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
k8s.io/utils v0.0.0-20230726121419-3b25d923346b h1:sgn3ZU783SCgtaSJjpcVVlRqd6GSnlTLKgpAAttJvpI=
k8s.io/utils v0.0.0-20230726121419-3b25d923346b/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/install_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ func requireCreatingKafkaCluster(kubectlOptions k8s.KubectlOptions, manifestPath
By(fmt.Sprintf("KafkaCluster %s already exists\n", kafkaClusterName))
} else {
By("Deploying a KafkaCluster")
applyK8sResourceManifest(kubectlOptions, manifestPath)
err := applyK8sResourceManifest(kubectlOptions, manifestPath)
Expect(err).NotTo(HaveOccurred())
}

By("Verifying the KafkaCluster state")
Expand Down
80 changes: 50 additions & 30 deletions tests/e2e/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,58 @@ import (
const (
// allowedCRDByteCount is the limitation of the number of bytes a CRD is
// allowed to have when being applied by K8s API server/kubectl.
allowedCRDByteCount = 262144 //nolint:unused // Note: this const is currently only used in helper functions which are not yet called on so this linter transitively fails for this const
allowedCRDByteCount = 262144 //nolint:unused // Note: this const is currently only used in helper functions which are not yet called on so this linter transitively fails for this const.

// crdNamePrefix is the prefix of the CRD names when listed through kubectl.
crdNamePrefix = "customresourcedefinition.apiextensions.k8s.io/" //nolint:unused // Note: this const is currently only used in helper functions which are not yet called on so this linter transitively fails for this const
crdNamePrefix = "customresourcedefinition.apiextensions.k8s.io/" //nolint:unused // Note: this const is currently only used in helper functions which are not yet called on so this linter transitively fails for this const.

// Per the kubectl spec, "dry-run" strategy must be "none", "server", or "client".
// With current use cases in e2e tests, we only expect to use "server" so the other options are commented out.
dryRunStrategyArgServer string = "--dry-run=server" // submit server-side request without persisting the resource.
//dryRunStrategyArgClient string = "--dry-run=client" // the resource is only validated locally but not sent to the Api-server.
//dryRunStrategyArgNone string = "--dry-run=none" // default value; submit server-side request and persist the resource.

)

// applyK8sResourceManifests applies the specified manifest to the provided
// kubectl context and namespace.
func applyK8sResourceManifest(kubectlOptions k8s.KubectlOptions, manifestPath string) { //nolint:unused // Note: this might come in handy for manual K8s resource operations.
By(fmt.Sprintf("Applying k8s manifest %s", manifestPath))
k8s.KubectlApply(GinkgoT(), &kubectlOptions, manifestPath)
func applyK8sResourceManifest(kubectlOptions k8s.KubectlOptions, manifestPath string, extraArgs ...string) error { //nolint:unused // Note: this might come in handy for manual K8s resource operations.
args := []string{"apply", "-f", manifestPath}
logMsg := fmt.Sprintf("Applying k8s manifest from path %s", manifestPath)
logMsg, args = kubectlArgExtender(args, logMsg, "", "", kubectlOptions.Namespace, extraArgs)
By(logMsg)

return k8s.RunKubectlE(GinkgoT(), &kubectlOptions, args...)
}

// applyK8sResourceManifestFromString applies the specified manifest in string format to the provided
// kubectl context and namespace.
func applyK8sResourceManifestFromString(kubectlOptions k8s.KubectlOptions, manifest string, extraArgs ...string) error {
tmpfile, err := createTempFileFromBytes([]byte(manifest), "", "", 0)
if err != nil {
return fmt.Errorf("storing provided manifest data into temp file failed: %w", err)
}
defer os.Remove(tmpfile)

return applyK8sResourceManifest(kubectlOptions, tmpfile, extraArgs...)
}

// applyK8sResourceFromTemplate generates manifest from the specified go-template based on values
// and applies the specified manifest to the provided kubectl context and namespace.
func applyK8sResourceFromTemplate(kubectlOptions k8s.KubectlOptions, templateFile string, values any, extraArgs ...string) error {
By(fmt.Sprintf("Generating k8s manifest from template %s", templateFile))
var manifest bytes.Buffer
rawTemplate, err := os.ReadFile(templateFile)
if err != nil {
return err
}
t := template.Must(template.New("template").Funcs(sprig.TxtFuncMap()).Parse(string(rawTemplate)))
err = t.Execute(&manifest, values)
if err != nil {
return err
}

return applyK8sResourceManifestFromString(kubectlOptions, manifest.String(), extraArgs...)
}

// isExistingK8SResource queries a Resource by it's kind, namespace and name and
Expand Down Expand Up @@ -191,7 +232,10 @@ func installK8sCRD(kubectlOptions k8s.KubectlOptions, crd []byte, shouldBeValida

createOrReplaceK8sResourcesFromManifest(kubectlOptions, "crd", object.GetName(), tempPath, shouldBeValidated)
default: // Note: regular CRD.
applyK8sResourceManifest(kubectlOptions, tempPath)
err = applyK8sResourceManifest(kubectlOptions, tempPath)
if err != nil {
return errors.WrapIfWithDetails(err, "applying CRD failed", "crd", string(crd))
Copy link
Contributor

@bartam1 bartam1 Aug 23, 2023

Choose a reason for hiding this comment

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

Note:
When we use the emperror.dev/errors's WithDetails feature then IMHO the error message will not contain the details because it is stored in a separate structure member variable and not in the error message itself.

I think we should figure out something to solve this problem.
I favor the std Go error handling with the fmt.Errorf but I know the stacktrace will be not there. (it is also stored in a separate struct member variable in emperror case)

I know we use that lot of places in the e2e codebase

}
}

return nil
Expand Down Expand Up @@ -453,30 +497,6 @@ func deleteK8sResourceNoErrNotFound(kubectlOptions k8s.KubectlOptions, timeout t
return err
}

// applyK8sResourceManifestFromString applies the specified manifest in string format to the provided
// kubectl context and namespace.
func applyK8sResourceManifestFromString(kubectlOptions k8s.KubectlOptions, manifest string) error {
By(fmt.Sprintf("Applying k8s manifest\n%s", manifest))
return k8s.KubectlApplyFromStringE(GinkgoT(), &kubectlOptions, manifest)
}

// applyK8sResourceFromTemplate generates manifest from the specified go-template based on values
// and applies the specified manifest to the provided kubectl context and namespace.
func applyK8sResourceFromTemplate(kubectlOptions k8s.KubectlOptions, templateFile string, values map[string]interface{}) error {
By(fmt.Sprintf("Generating K8s manifest from template %s", templateFile))
var manifest bytes.Buffer
rawTemplate, err := os.ReadFile(templateFile)
if err != nil {
return err
}
t := template.Must(template.New("template").Funcs(sprig.TxtFuncMap()).Parse(string(rawTemplate)))
err = t.Execute(&manifest, values)
if err != nil {
return err
}
return applyK8sResourceManifestFromString(kubectlOptions, manifest.String())
}

// listK8sResourceKinds lists all of the available resource kinds on the K8s cluster
// with the apiGroupSelector parameter the result can be narrowed by the resource group.
// extraArgs can be any kubectl api-resources parameter.
Expand Down
42 changes: 35 additions & 7 deletions tests/e2e/kafka.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/gruntwork-io/terratest/modules/k8s"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"k8s.io/utils/ptr"
)

// requireDeleteKafkaTopic deletes kafkaTopic resource.
Expand All @@ -34,14 +35,21 @@ func requireDeleteKafkaTopic(kubectlOptions k8s.KubectlOptions, topicName string
// requireDeployingKafkaTopic deploys a kafkaTopic resource from a template
func requireDeployingKafkaTopic(kubectlOptions k8s.KubectlOptions, topicName string) {
It("Deploying KafkaTopic CR", func() {
err := applyK8sResourceFromTemplate(kubectlOptions,
kafkaTopicTemplate,
map[string]interface{}{
"Name": topicName,
"TopicName": topicName,
"Namespace": kubectlOptions.Namespace,
values := kafkaTopicTemplateData{
Annotations: []string{"managedBy: koperator"},
ClusterRef: kafkaTopicClusterRef{
Name: kafkaClusterName,
Namespace: kubectlOptions.Namespace,
},
)
Name: topicName,
Namespace: kubectlOptions.Namespace,
Partitions: ptr.To(int32(2)),
ReplicationFactor: ptr.To(int32(2)),
TopicName: topicName,
}

err := applyK8sResourceFromTemplate(kubectlOptions, kafkaTopicTemplate, values)

Expect(err).ShouldNot(HaveOccurred())

err = waitK8sResourceCondition(kubectlOptions, kafkaTopicKind,
Expand Down Expand Up @@ -82,3 +90,23 @@ func requireDeployingKafkaUser(kubectlOptions k8s.KubectlOptions, userName strin
}, defaultUserCreationWaitTime, 3*time.Second).Should(Equal(true))
})
}

// kafkaTopicTemplateData is a struct that holds the relevant information and structure
// to fill out the template used to generate KafkaTopics.
// TODO: long term we should use the structs in the api module instead of these local structs.
type kafkaTopicTemplateData struct {
Annotations []string
ClusterRef kafkaTopicClusterRef
Name string
Namespace string
Partitions *int32
ReplicationFactor *int32
TopicName string
}

// kafkaTopicClusterRef holds the information relevant to identifying a KafkaCluster within a KafkaTopic CR.
// TODO: Long term, we should use the structs in the api module instead of these local structs.
type kafkaTopicClusterRef struct {
Name string
Namespace string
}
Loading
Loading