Skip to content

Commit

Permalink
scorecard,generate-csv: consider api "group" while checking for known…
Browse files Browse the repository at this point in the history
… types

Comparing only "kind" of the resource might lead to conflict where two
resources might have same kind but in different api groups.

For example an operator could provide a custom resource with name
"Deployment" in it's own group. On generating csv for this operator
might not consider the provided CR for filling 'alm-examples'.
  • Loading branch information
avalluri committed Jul 6, 2020
1 parent b1c31f7 commit 8ff664a
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 16 deletions.
5 changes: 5 additions & 0 deletions changelog/fragments/crds-with-core-types.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
entries:
- description: >
Fix bug that occurs to fill the example in the process to generate the CSV when the Operator API has Kinds named as core types by starting to check its groups.
kind: "bugfix"
breaking: false
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,37 @@ var _ = Describe("Generating a ClusterServiceVersion", func() {
Expect(csv).To(Equal(upgradeCSV(newCSV, g.OperatorName, g.Version)))
})
})

Context("generate ClusterServiceVersion", func() {
It("should handle CRDs with core type name", func() {
g = Generator{
OperatorName: operatorName,
OperatorType: operatorType,
Version: version,
Collector: &collector.Manifests{},
config: cfg,
getBase: makeBaseGetter(newCSV),
}
err := filepath.Walk(goConfigDir, func(path string, info os.FileInfo, err error) error {
if err != nil || info.IsDir() {
return err
}
file, err := os.OpenFile(path, os.O_RDONLY, 0)
if err != nil {
return err
}
defer file.Close()
return g.Collector.UpdateFromReader(file)
})
Expect(err).ShouldNot(HaveOccurred(), "failed to read manifests")
Expect(len(g.Collector.V1beta1CustomResourceDefinitions)).Should(BeEquivalentTo(2))
Expect(len(g.Collector.CustomResources)).Should(BeEquivalentTo(2))

csv, err := g.generate()
Expect(err).ToNot(HaveOccurred())
Expect(csv.Annotations["alm-examples"]).ShouldNot(BeEquivalentTo("[]"))
})
})
})

})
Expand Down
37 changes: 23 additions & 14 deletions internal/generate/collector/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ type Manifests struct {
Others []unstructured.Unstructured
}

var (
roleGK = rbacv1.SchemeGroupVersion.WithKind("Role").GroupKind()
clusterRoleGK = rbacv1.SchemeGroupVersion.WithKind("ClusterRole").GroupKind()
deploymentGK = appsv1.SchemeGroupVersion.WithKind("Deployment").GroupKind()
crdGK = apiextv1.SchemeGroupVersion.WithKind("CustomResourceDefinition").GroupKind()
validatingWebhookCfgGK = admissionregv1.SchemeGroupVersion.WithKind("ValidatingWebhookConfiguration").GroupKind()
mutatingWebhookCfgGK = admissionregv1.SchemeGroupVersion.WithKind("MutatingWebhookConfiguration").GroupKind()
)

// UpdateFromDirs adds Roles, ClusterRoles, Deployments, and Custom Resource examples
// found in deployDir, and CustomResourceDefinitions found in crdsDir,
// to their respective fields in a Manifests, then filters and deduplicates them.
Expand All @@ -72,18 +81,18 @@ func (c *Manifests) UpdateFromDirs(deployDir, crdsDir string) error {
}

gvk := typeMeta.GroupVersionKind()
switch gvk.Kind {
case "Role":
switch gvk.GroupKind() {
case roleGK:
err = c.addRoles(manifest)
case "ClusterRole":
case clusterRoleGK:
err = c.addClusterRoles(manifest)
case "Deployment":
case deploymentGK:
err = c.addDeployments(manifest)
case "CustomResourceDefinition":
case crdGK:
// Skip for now and add explicitly from CRDsDir input.
case "ValidatingWebhookConfiguration":
case validatingWebhookCfgGK:
err = c.addValidatingWebhookConfigurations(manifest)
case "MutatingWebhookConfiguration":
case mutatingWebhookCfgGK:
err = c.addMutatingWebhookConfigurations(manifest)
default:
err = c.addOthers(manifest)
Expand Down Expand Up @@ -133,18 +142,18 @@ func (c *Manifests) UpdateFromReader(r io.Reader) error {
}

gvk := typeMeta.GroupVersionKind()
switch gvk.Kind {
case "Role":
switch gvk.GroupKind() {
case roleGK:
err = c.addRoles(manifest)
case "ClusterRole":
case clusterRoleGK:
err = c.addClusterRoles(manifest)
case "Deployment":
case deploymentGK:
err = c.addDeployments(manifest)
case "CustomResourceDefinition":
case crdGK:
err = c.addCustomResourceDefinitions(gvk.Version, manifest)
case "ValidatingWebhookConfiguration":
case validatingWebhookCfgGK:
err = c.addValidatingWebhookConfigurations(manifest)
case "MutatingWebhookConfiguration":
case mutatingWebhookCfgGK:
err = c.addMutatingWebhookConfigurations(manifest)
default:
err = c.addOthers(manifest)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: deployment.foo.example.com
spec:
group: foo.example.com
names:
kind: Deployment
listKind: DeploymentList
plural: deployments
singular: deployment
scope: Namespaced
subresources:
status: {}
validation:
openAPIV3Schema:
description: Schema for the deployments API
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
spec:
description: DeploymentSpec defines the desired state of the Foo Deployment
properties:
size:
description: Size is the size of the Foo deployment nodes
format: int32
type: integer
required:
- size
type: object
status:
description: DeploymentStatus defines the observed state of the Foo Deployment
properties:
nodes:
description: Nodes are the names of the Foo pods
items:
type: string
type: array
required:
- nodes
type: object
type: object
version: v1alpha1
versions:
- name: v1alpha1
served: true
storage: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
apiVersion: foo.example.com/v1alpha1
kind: Deployment
metadata:
name: example-foo-deployment
spec:
# Add fields here
size: 3
4 changes: 2 additions & 2 deletions internal/scorecard/plugins/resource_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func createFromYAMLFile(cfg BasicAndOLMPluginConfig, yamlPath string) error {
obj.SetNamespace(cfg.Namespace)

// dirty hack to merge scorecard proxy into operator deployment; lots of serialization and deserialization
if obj.GetKind() == "Deployment" {
if obj.GroupVersionKind().Group == "apps" && obj.GetKind() == "Deployment" {
// TODO: support multiple deployments
if deploymentName != "" {
return fmt.Errorf("scorecard currently does not support multiple deployments in the manifests")
Expand Down Expand Up @@ -153,7 +153,7 @@ func createFromYAMLFile(cfg BasicAndOLMPluginConfig, yamlPath string) error {
}
}
addResourceCleanup(obj, types.NamespacedName{Namespace: obj.GetNamespace(), Name: obj.GetName()}, cfg.InitTimeout)
if obj.GetKind() == "Deployment" {
if obj.GroupVersionKind().Group == "apps" && obj.GetKind() == "Deployment" {
proxyPodGlobal, err = getPodFromDeployment(deploymentName, cfg.Namespace)
if err != nil {
return err
Expand Down

0 comments on commit 8ff664a

Please sign in to comment.