Skip to content

Commit

Permalink
Address feedbacks from comments/reviews
Browse files Browse the repository at this point in the history 
1. Improve errors handling
2. Flattering some nested conditional loops
3. Refactoring supported types
4. Handle plain type validation

Signed-off-by: Vu Dinh <vdinh@redhat.com>
  • Loading branch information
@dinhxuanvu
dinhxuanvu committed Jan 16, 2020
1 parent cbbe884 commit 72a7841
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 72 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Expand Up @@ -449,6 +449,7 @@ bin

# Ignore sqlite
*.db
*.db-journal

# Ignore vscode
.vscode
Expand Down
2 changes: 1 addition & 1 deletion cmd/opm/alpha/bundle/validate.go
View file
Expand Up @@ -62,7 +62,7 @@ func validateFunc(cmd *cobra.Command, args []string) error {
return err
}

err = imageValidator.ValidateBundleFormat(filepath.Join(dir, bundle.ManifestsDir))
err = imageValidator.ValidateBundleContent(filepath.Join(dir, bundle.ManifestsDir))
if err != nil {
return err
}
Expand Down
41 changes: 41 additions & 0 deletions pkg/lib/bundle/supported_resources.go
View file
@@ -0,0 +1,41 @@
package bundle

const (
CSVKind = "ClusterServiceVersion"
CRDKind = "CustomResourceDefinition"
SecretKind = "Secret"
ClusterRoleKind = "ClusterRole"
ClusterRoleBindingKind = "ClusterRoleBinding"
ServiceAccountKind = "ServiceAccount"
ServiceKind = "Service"
RoleKind = "Role"
RoleBindingKind = "RoleBinding"
PrometheusRuleKind = "PrometheusRule"
ServiceMonitorKind = "ServiceMonitor"
)

var supportedResources map[string]bool

// Add a list of supported resources to the map
// Key: Kind name
// Value: If namaspaced kind, true. Otherwise, false
func init() {
supportedResources = make(map[string]bool, 11)

supportedResources[CSVKind] = true
supportedResources[CRDKind] = false
supportedResources[ClusterRoleKind] = false
supportedResources[ClusterRoleBindingKind] = false
supportedResources[ServiceKind] = true
supportedResources[ServiceAccountKind] = true
supportedResources[RoleKind] = true
supportedResources[RoleBindingKind] = true
supportedResources[PrometheusRuleKind] = true
supportedResources[ServiceMonitorKind] = true
}

// IsSupported checks if the object kind is OLM-supported and if it is namespaced
func IsSupported(kind string) (bool, bool) {
namespaced, ok := supportedResources[kind]
return ok, namespaced
}
130 changes: 59 additions & 71 deletions pkg/lib/bundle/validate.go
View file
Expand Up @@ -25,18 +25,6 @@ import (
"gopkg.in/yaml.v2"
)

const (
csvKind = "ClusterServiceVersion"
crdKind = "CustomResourceDefinition"
secretKind = "Secret"
clusterRoleKind = "ClusterRole"
clusterRoleBindingKind = "ClusterRoleBinding"
serviceAccountKind = "ServiceAccount"
serviceKind = "Service"
roleKind = "Role"
roleBindingKind = "RoleBinding"
)

type Meta struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata"`
Expand Down Expand Up @@ -73,7 +61,11 @@ func (i imageValidator) ValidateBundleFormat(directory string) error {
var annotationsDir, manifestsDir string
var validationErrors []error

items, _ := ioutil.ReadDir(directory)
items, err := ioutil.ReadDir(directory)
if err != nil {
validationErrors = append(validationErrors, err)
}

for _, item := range items {
if item.IsDir() {
switch s := item.Name(); s {
Expand Down Expand Up @@ -200,29 +192,21 @@ func (i imageValidator) ValidateBundleContent(manifestDir string) error {
}

switch mediaType {
case HelmType, PlainType:
case HelmType:
return nil
}

supportedTypes := map[string]string{
csvKind: "",
crdKind: "",
secretKind: "",
clusterRoleKind: "",
clusterRoleBindingKind: "",
serviceKind: "",
serviceAccountKind: "",
roleKind: "",
roleBindingKind: "",
}

var csvName string
unstObjs := []*unstructured.Unstructured{}
csvValidator := v.ClusterServiceVersionValidator
crdValidator := v.CustomResourceDefinitionValidator

// Read all files in manifests directory
items, _ := ioutil.ReadDir(manifestDir)
items, err := ioutil.ReadDir(manifestDir)
if err != nil {
validationErrors = append(validationErrors, err)
}

for _, item := range items {
fileWithPath := filepath.Join(manifestDir, item.Name())
data, err := ioutil.ReadFile(fileWithPath)
Expand All @@ -233,50 +217,54 @@ func (i imageValidator) ValidateBundleContent(manifestDir string) error {

dec := k8syaml.NewYAMLOrJSONDecoder(strings.NewReader(string(data)), 30)
k8sFile := &unstructured.Unstructured{}
if err := dec.Decode(k8sFile); err == nil {
unstObjs = append(unstObjs, k8sFile)
kind := k8sFile.GetObjectKind().GroupVersionKind().Kind
i.logger.Debugf(`Validating file "%s" with type "%s"`, item.Name(), kind)
// Verify if the object kind is supported for registryV1 format
if _, ok := supportedTypes[kind]; !ok {
validationErrors = append(validationErrors, fmt.Errorf("%s is not supported type for registryV1 bundle: %s", kind, fileWithPath))
} else {
if kind == csvKind {
csv := &v1.ClusterServiceVersion{}
err := runtime.DefaultUnstructuredConverter.FromUnstructured(k8sFile.Object, csv)
if err != nil {
validationErrors = append(validationErrors, err)
}

csvName = csv.GetName()
results := csvValidator.Validate(csv)
if len(results) > 0 {
for _, err := range results[0].Errors {
validationErrors = append(validationErrors, err)
}
}
} else if kind == crdKind {
crd := &v1beta1.CustomResourceDefinition{}
err := runtime.DefaultUnstructuredConverter.FromUnstructured(k8sFile.Object, crd)
if err != nil {
validationErrors = append(validationErrors, err)
}

results := crdValidator.Validate(crd)
if len(results) > 0 {
for _, err := range results[0].Errors {
validationErrors = append(validationErrors, err)
}
}
} else {
err := validateKubectlable(data)
if err != nil {
validationErrors = append(validationErrors, err)
}
err = dec.Decode(k8sFile)
if err != nil {
validationErrors = append(validationErrors, err)
continue
}

unstObjs = append(unstObjs, k8sFile)
gvk := k8sFile.GetObjectKind().GroupVersionKind()
i.logger.Debugf(`Validating "%s" from file "%s"`, gvk.String(), item.Name())
// Verify if the object kind is supported for RegistryV1 format
ok, _ := IsSupported(gvk.Kind)
if mediaType == RegistryV1Type && !ok {
validationErrors = append(validationErrors, fmt.Errorf("%s is not supported type for registryV1 bundle: %s", gvk.Kind, fileWithPath))
continue
}

if gvk.Kind == CSVKind {
csv := &v1.ClusterServiceVersion{}
err := runtime.DefaultUnstructuredConverter.FromUnstructured(k8sFile.Object, csv)
if err != nil {
validationErrors = append(validationErrors, err)
}

csvName = csv.GetName()
results := csvValidator.Validate(csv)
if len(results) > 0 {
for _, err := range results[0].Errors {
validationErrors = append(validationErrors, err)
}
}
} else if gvk.Kind == CRDKind {
crd := &v1beta1.CustomResourceDefinition{}
err := runtime.DefaultUnstructuredConverter.FromUnstructured(k8sFile.Object, crd)
if err != nil {
validationErrors = append(validationErrors, err)
}

results := crdValidator.Validate(crd)
if len(results) > 0 {
for _, err := range results[0].Errors {
validationErrors = append(validationErrors, err)
}
}
} else {
validationErrors = append(validationErrors, err)
err := validateKubectlable(data)
if err != nil {
validationErrors = append(validationErrors, err)
}
}
}

Expand All @@ -301,13 +289,13 @@ func (i imageValidator) ValidateBundleContent(manifestDir string) error {

// Validate if the file is kubecle-able
func validateKubectlable(fileBytes []byte) error {
exampleFileBytesJson, err := y.YAMLToJSON(fileBytes)
exampleFileBytesJSON, err := y.YAMLToJSON(fileBytes)
if err != nil {
return err
}

parsedMeta := &Meta{}
err = json.Unmarshal(exampleFileBytesJson, parsedMeta)
err = json.Unmarshal(exampleFileBytesJSON, parsedMeta)
if err != nil {
return err
}
Expand All @@ -322,7 +310,7 @@ func validateKubectlable(fileBytes []byte) error {
)

if len(errs) > 0 {
return fmt.Errorf("error validating object metadata: %s. %v", errs, parsedMeta)
return fmt.Errorf("error validating object metadata: %s. %v", errs.ToAggregate(), parsedMeta)
}

return nil
Expand Down

0 comments on commit 72a7841

Please sign in to comment.