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

Add validations for customresourcedefinitions and apiservicedefinitions #112

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add validations for customresourcedefinitions and apiservicedefinitions #112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ce5ce69
validate: validate csv spec.crd.owned & descriptors
darkowlzz Apr 23, 2019
07c881a
validate: add validation for apiservicedefinitions
darkowlzz Apr 23, 2019
983e781
Remove owned requirement
darkowlzz May 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 132 additions & 13 deletions operatorcourier/validate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,21 +186,28 @@ def _csv_spec_validation(self, spec, bundleData):
valid = False

if "customresourcedefinitions" in spec:
customresourcedefinitions = spec["customresourcedefinitions"]
if self._csv_crd_validation(
spec["customresourcedefinitions"], bundleData) is False:
valid = False

crdList = []
for crd in bundleData[self.crdKey]:
try:
name = crd["metadata"]["name"]
crdList.append(name)
except KeyError:
pass

if "owned" not in customresourcedefinitions:
self._log_error("spec.customresourcedefinitions.owned"
"not defined for csv")
return False
if "apiservicedefinitions" in spec:
if self._csv_asd_validation(spec["apiservicedefinitions"]) is False:
valid = False

return valid

def _csv_crd_validation(self, customresourcedefinitions, bundleData):
valid = True

crdList = []
for crd in bundleData[self.crdKey]:
try:
name = crd["metadata"]["name"]
crdList.append(name)
except KeyError:
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this log a warning or error informing the user that a CRD is missing the metadata.name field? It's better to be noisy than quiet in this case (unless we previously warn elsewhere)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's logged here https://github.com/operator-framework/operator-courier/blob/v2.0.2/operatorcourier/validate.py#L107


if "owned" in customresourcedefinitions:
for csvOwnedCrd in customresourcedefinitions["owned"]:
if "name" not in csvOwnedCrd:
self._log_error("name not defined for item in "
Expand All @@ -221,6 +228,46 @@ def _csv_spec_validation(self, spec, bundleData):
"spec.customresourcedefinitions.")
valid = False

# Values of name, version and kind above are compared with their
# values in the associated CRD files later. Empty string check
# is not needed.
# displayName and description should be checked for empty
# strings.
if "displayName" not in csvOwnedCrd:
self._log_error("displayName not defined for item in "
"spec.customresourcedefinitions.")
valid = False
elif not csvOwnedCrd["displayName"]:
self._log_error("displayName is empty for item in "
"spec.customresourcedefinitions.")
valid = False
if "description" not in csvOwnedCrd:
self._log_error("description not defined for item in "
"spec.customresourcedefinitions.")
valid = False
elif not csvOwnedCrd["description"]:
self._log_error("description is empty for item in "
"spec.customresourcedefinitions.")
valid = False

if "specDescriptors" in csvOwnedCrd and "name" in csvOwnedCrd:
if self._csv_descriptors_validation(
csvOwnedCrd["specDescriptors"],
csvOwnedCrd["name"]) is False:
valid = False

if "statusDescriptors" in csvOwnedCrd and "name" in csvOwnedCrd:
if self._csv_descriptors_validation(
csvOwnedCrd["statusDescriptors"],
csvOwnedCrd["name"]) is False:
valid = False

if "actionDescriptors" in csvOwnedCrd and "name" in csvOwnedCrd:
if self._csv_descriptors_validation(
csvOwnedCrd["actionDescriptors"],
csvOwnedCrd["name"]) is False:
valid = False

for crd in bundleData[self.crdKey]:
if 'name' not in csvOwnedCrd:
continue
Expand Down Expand Up @@ -259,6 +306,53 @@ def _csv_spec_validation(self, spec, bundleData):
"match "
"CSV.spec.crd.owned.name")
valid = False

return valid

def _csv_asd_validation(self, apiservicedefinitions):
valid = True

if "owned" in apiservicedefinitions:
# required attributes of owned apiservicedefinitions
attributeList = ["group", "version", "kind", "name", "deploymentName",
"displayName", "description"]

# validate the owned apiservicedefinitions
def validate_owned(resource, attribute):
if attribute not in resource:
self._log_error(
"%s not defined for item in spec.apiservicedefinitions."
% attribute)
return False
elif not resource[attribute]:
self._log_error("%s is empty for item in "
"spec.apiservicedefinitions." % attribute)
return False
return True

for csvOwnedAsd in apiservicedefinitions["owned"]:
for attr in attributeList:
if validate_owned(csvOwnedAsd, attr) is False:
valid = False

if "specDescriptors" in csvOwnedAsd and "name" in csvOwnedAsd:
if self._csv_descriptors_validation(
csvOwnedAsd["specDescriptors"],
csvOwnedAsd["name"]) is False:
valid = False

if "statusDescriptors" in csvOwnedAsd and "name" in csvOwnedAsd:
if self._csv_descriptors_validation(
csvOwnedAsd["statusDescriptors"],
csvOwnedAsd["name"]) is False:
valid = False

if "actionDescriptors" in csvOwnedAsd and "name" in csvOwnedAsd:
if self._csv_descriptors_validation(
csvOwnedAsd["actionDescriptors"],
csvOwnedAsd["name"]) is False:
valid = False

return valid

def _csv_spec_install_validation(self, install):
Expand Down Expand Up @@ -314,6 +408,31 @@ def _csv_spec_install_validation(self, install):

return valid

def _csv_descriptors_validation(self, descriptors, crdName):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also check that the x-descriptors (if it exists) field is a list of strings?
Eq: https://github.com/operator-framework/community-operators/blob/master/community-operators/cluster-logging/cluster-logging.v4.1.0.clusterserviceversion.yaml#L275
Not sure if this is a requirement, more like an observation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, thanks for the idea, that would be useful and I would like to add that in a separate PR. This PR is already huge.

valid = True

# required attributes of descriptors
attributeList = ["displayName", "description", "path"]

# validate a descriptor for a given attribute
def validate_descriptor(descriptor, resourceName, attribute):
if attribute not in descriptor:
self._log_error("%s is not defined for descriptors in %s"
% (attribute, resourceName))
return False
elif not descriptor[attribute]:
self._log_error("%s is empty for descriptors in %s"
% (attribute, resourceName))
return False
return True

for desc in descriptors:
for attr in attributeList:
if validate_descriptor(desc, crdName, attr) is False:
valid = False

return valid

def _csv_metadata_validation(self, metadata):
valid = True

Expand Down
Loading