Skip to content

Commit

Permalink
Refine input validation wording for annotations
Browse files Browse the repository at this point in the history
  • Loading branch information
gberche-orange committed Jul 13, 2021
1 parent f45cc0e commit 57d5b7a
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 15 deletions.
28 changes: 26 additions & 2 deletions osb-cmdb/docs/impl-notes/issue-108-osb-context-annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -483,12 +483,36 @@ Alternative fixes:
* "orange.com/a-key" annotation found in context organization_annotations has unsupported value "space is not supported"
* reproduce in unit test
* "orange.com/a-very-long-key" annotation found in context organization_annotations is length longer than supported (i.e. max label - osb-cmdb key prefix)

* Perform validation on labels prior to saving them and fail with internal error
* [x] End to end test
* [ ] Add acceptance test
* [x] Manual test
```
$ cf create-service noop-ondemand default osb-cmdb-broker-0-smoketest-1626184294 -b osb-cmdb-broker-0
Creating service instance osb-cmdb-broker-0-smoketest-1626184294 in org osb-cmdb-brokered-services-org-client-0 / space smoke-tests as admin...
Service broker error: Service broker parameters are invalid: Annotation key "orange.com/key-with-chars-incompatible-with-labels" with value "a key with spaces" can not be indexed in osb-cmdb as a label "key-with-chars-incompatible-with-labels" due to violations to regex :[a-z0-9A-Z\-_\.]{0,63}
FAILED
```


* ~~Perform validation on labels prior to saving them and fail with internal error~~ Overlaps with the 2 other supports.
* MetaData object with javax validation constraints
* [x] Refine error handling on metadata update to fail fast on this condition
* [x] wrap into our own exception: subclass of ServiceBrokerException: OsbCmdbInternalErrorException
* might leak some underlying problem ?
* at least not in the current example `org.cloudfoundry.client.v3.ClientV3Exception: CF-UnprocessableEntity(10008): Metadata label key error: 'brokered...' is greater than 63 characters, Metadata label value error: 'a key with spaces' contains invalid characters`, still redact it
* is insufficient to provide meaningful user-facing diagnostic
* [x] Manual test
```
$ cf create-service noop-ondemand default osb-cmdb-broker-0-smoketest-1626172686 -b osb-cmdb-broker-0
Creating service instance osb-cmdb-broker-0-smoketest-1626172686 in org osb-cmdb-brokered-services-org-client-0 / space smoke-tests as admin...
Service broker error: org.cloudfoundry.client.v3.ClientV3Exception: CF-UnprocessableEntity(10008): Metadata label key error: 'brokered...' is greater than 63 characters, Metadata label value error: 'a key with spaces' contains invalid characters
FAILED
```
* [ ] Add acceptance test

Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,18 @@ public void validateOrangeAnnotationCanBeIndexedAsALabel(String userDefinedAnnot
String userDefinedAnnotationValue,
String trimmedKey) {
//See https://docs.cloudfoundry.org/adminguide/metadata.html#reqs
String wrappedKey = BROKERED_SERVICE_CONTEXT_ORANGE + trimmedKey;
if (!trimmedKey.matches(LABEL_KEY_REGEX)) {
throw new ServiceBrokerInvalidParametersException("Annotation key \"" + userDefinedAnnotationKey + "\" " +
"can not be indexed in osb-cmdb as a label \"" + trimmedKey + "\" due to violations to regex " +
"can not be indexed in osb-cmdb as a label \"" + wrappedKey + "\" due" +
" to violations to regex " +
":" + LABEL_KEY_REGEX + " (please check maxsize for annotation key =" + (63- BROKERED_SERVICE_CONTEXT_ORANGE.length()) +
" chars)");
}
if (!userDefinedAnnotationValue.matches(LABEL_VALUE_REGEX)) {
throw new ServiceBrokerInvalidParametersException("Annotation key \"" + userDefinedAnnotationKey + "\" " +
"with value \"" + userDefinedAnnotationValue + "\" " +
"can not be indexed in osb-cmdb as a label \"" + trimmedKey + "\" due to violations to regex " +
"can not be indexed in osb-cmdb as a label \"" + wrappedKey + "\" due to value violation to regex " +
":" + LABEL_VALUE_REGEX);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,47 +17,49 @@ void accepts_valid_key_values() {
//given
String userDefinedAnnotationKey = "orange.com/a-key_name.suffix";
String value = "a-valid-key";
String wrappedLabelKey = BROKERED_SERVICE_CONTEXT_ORANGE + "a-key_name.suffix";
String trimmedLabelKey = "a-key_name.suffix";

//when
validator.validateOrangeAnnotationCanBeIndexedAsALabel(userDefinedAnnotationKey, value, wrappedLabelKey);
validator.validateOrangeAnnotationCanBeIndexedAsALabel(userDefinedAnnotationKey, value, trimmedLabelKey);

//then no exeption is thrown
}

@Test
void rejects_invalid_key() {
//given
String userDefinedAnnotationKey = "orange.com/key-with-chars-incompatible-with-labels";
String value = "a key with spaces";
String wrappedLabelKey = BROKERED_SERVICE_CONTEXT_ORANGE + "key-with-chars-incompatible-with-labels";
String userDefinedAnnotationKey = "orange.com/key with spaces";
String value = "a-valid-value";
String trimmedLabelKey = "key with spaces";

//when
ServiceBrokerInvalidParametersException exception = assertThrows(ServiceBrokerInvalidParametersException.class, () -> {
validator.validateOrangeAnnotationCanBeIndexedAsALabel(userDefinedAnnotationKey, value,
wrappedLabelKey);
trimmedLabelKey);
});

//then
String expectedMessage = "Service broker parameters are invalid: Annotation key \"orange.com/key-with-chars-incompatible-with-labels\" can not be indexed in osb-cmdb as a label \"brokered_service_context_orange_key-with-chars-incompatible-with-labels\" due to violations to regex :[a-z0-9A-Z\\-_\\.]{1,63} (please check maxsize for annotation key =31 chars)";
String expectedMessage = "Service broker parameters are invalid: Annotation key \"orange.com/key with spaces\" can not be indexed in osb-cmdb as a label \"brokered_service_context_orange_key with spaces\" due to violations to regex :[a-z0-9A-Z\\-_\\.]{1,63} (please check maxsize for annotation key =31 chars)";
assertThat(exception.getMessage()).isEqualTo(expectedMessage);
}

@Test
void rejects_invalid_value() {
//given
String userDefinedAnnotationKey = "orange.com/key";
String wrappedLabelKey = BROKERED_SERVICE_CONTEXT_ORANGE + "key";
String trimmedLabelKey = "key";
String value = "a key with spaces";

//when
ServiceBrokerInvalidParametersException exception = assertThrows(ServiceBrokerInvalidParametersException.class, () -> {
validator.validateOrangeAnnotationCanBeIndexedAsALabel(userDefinedAnnotationKey, value,
wrappedLabelKey);
trimmedLabelKey);
});

//then
String expectedMessage = "Service broker parameters are invalid: Annotation key \"orange.com/key\" with value \"a key with spaces\" can not be indexed in osb-cmdb as a label \"brokered_service_context_orange_key\" due to violations to regex :[a-z0-9A-Z\\-_\\.]{0,63}";
String expectedMessage = "Service broker parameters are invalid: Annotation key \"orange.com/key\" with value" +
" \"a key with spaces\" can not be indexed in osb-cmdb as a label \"brokered_service_context_orange_key\"" +
" due to value violation to regex :[a-z0-9A-Z\\-_\\.]{0,63}";
assertThat(exception.getMessage()).isEqualTo(expectedMessage);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ void rejects_orange_annotations_that_cannot_be_indexed_as_labels() {
createServiceMetadataFormatterService.formatAsMetadata(request);
});

assertThat(exception.getMessage()).contains("due to violations to regex");
assertThat(exception.getMessage()).contains("due to value violation to regex");
}

private CreateServiceInstanceRequest aCreateRequestWithAnnotations(HashMap<String, Object> organizationAnnotations,
Expand Down

0 comments on commit 57d5b7a

Please sign in to comment.