Merge pull request #9962 from backstage/freben/validators

update the common validators

.changeset/slow-vans-rhyme.md

@@ -0,0 +1,5 @@
+---
+'@backstage/catalog-model': minor
+---
+
+**BREAKING**: The default validator for `metadata.tags` now permits the colon (`:`) character as well.

.changeset/ten-rats-join.md

@@ -0,0 +1,8 @@
+---
+'@backstage/catalog-model': patch
+---
+
+**DEPRECATION**:
+
+- Deprecated `CommonValidatorFunctions.isValidString`, please use `isNonEmptyString` instead which is equivalent but better named.
+- Deprecated `CommonValidatorFunctions.isValidTag`, with no replacement. Its purpose was too specific and not reusable, so it will be removed.

packages/catalog-model/api-report.md

@@ -51,6 +51,7 @@ export const apiEntityV1alpha1Validator: KindValidator;
 // @public
 export class CommonValidatorFunctions {
   static isJsonSafe(value: unknown): boolean;
+  static isNonEmptyString(value: unknown): value is string;
   static isValidDnsLabel(value: unknown): boolean;
   static isValidDnsSubdomain(value: unknown): boolean;
   static isValidPrefixAndOrSuffix(
@@ -59,7 +60,9 @@ export class CommonValidatorFunctions {
     isValidPrefix: (value: string) => boolean,
     isValidSuffix: (value: string) => boolean,
   ): boolean;
+  // @deprecated
   static isValidString(value: unknown): boolean;
+  // @deprecated
   static isValidTag(value: unknown): boolean;
   static isValidUrl(value: unknown): boolean;
 }

packages/catalog-model/src/entity/policies/FieldFormatEntityPolicy.ts

@@ -95,6 +95,7 @@ export class FieldFormatEntityPolicy implements EntityPolicy {
             expectation = 'a string that is a valid url';
             break;
           case 'isValidString':
+          case 'isNonEmptyString':
             expectation = 'a non empty string';
             break;
           default:
@@ -156,7 +157,7 @@ export class FieldFormatEntityPolicy implements EntityPolicy {
       optional(
         `links.${i}.title`,
         links[i]?.title,
-        CommonValidatorFunctions.isValidString,
+        CommonValidatorFunctions.isNonEmptyString,
       );
       optional(
         `links.${i}.icon`,

packages/catalog-model/src/validation/CommonValidatorFunctions.test.ts

@@ -226,4 +226,20 @@ describe('CommonValidatorFunctions', () => {
   ])(`isValidString %p ? %p`, (value, result) => {
     expect(CommonValidatorFunctions.isValidString(value)).toBe(result);
   });
+
+  it.each([
+    [null, false],
+    [true, false],
+    [7, false],
+    [{}, false],
+    ['', false],
+    [' ', false],
+    ['    ', false],
+    ['abc', true],
+    [' abc ', true],
+    ['abc xyz', true],
+    ['abc xyz abc.', true],
+  ])(`isNonEmptyString %p ? %p`, (value, result) => {
+    expect(CommonValidatorFunctions.isNonEmptyString(value)).toBe(result);
+  });
 });

packages/catalog-model/src/validation/CommonValidatorFunctions.ts

@@ -98,6 +98,7 @@ export class CommonValidatorFunctions {
   /**
    * Checks that the value is a valid tag.
    *
+   * @deprecated This will be removed in a future release
    * @param value - The value to check
    */
   static isValidTag(value: unknown): boolean {
@@ -110,7 +111,7 @@ export class CommonValidatorFunctions {
   }
 
   /**
-   * Checks that the value is a valid URL.
+   * Checks that the value is a valid string URL.
    *
    * @param value - The value to check
    */
@@ -131,9 +132,19 @@ export class CommonValidatorFunctions {
   /**
    * Checks that the value is a non empty string value.
    *
+   * @deprecated use isNonEmptyString instead
    * @param value - The value to check
    */
   static isValidString(value: unknown): boolean {
     return typeof value === 'string' && value?.trim()?.length >= 1;
   }
+
+  /**
+   * Checks that the value is a string value that's not empty.
+   *
+   * @param value - The value to check
+   */
+  static isNonEmptyString(value: unknown): value is string {
+    return typeof value === 'string' && value?.trim()?.length >= 1;
+  }
 }

packages/catalog-model/src/validation/makeValidator.ts

@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import { CommonValidatorFunctions } from './CommonValidatorFunctions';
 import { KubernetesValidatorFunctions } from './KubernetesValidatorFunctions';
 import { Validators } from './types';
 
@@ -27,7 +26,15 @@ const defaultValidators: Validators = {
   isValidLabelValue: KubernetesValidatorFunctions.isValidLabelValue,
   isValidAnnotationKey: KubernetesValidatorFunctions.isValidAnnotationKey,
   isValidAnnotationValue: KubernetesValidatorFunctions.isValidAnnotationValue,
-  isValidTag: CommonValidatorFunctions.isValidTag,
+  isValidTag: (value: unknown): boolean => {
+    // NOTE(freben): This one is a bit of an oddball and doesn't fit well anywhere to delegate to, so it's just inlined for now.
+    return (
+      typeof value === 'string' &&
+      value.length >= 1 &&
+      value.length <= 63 &&
+      /^[a-z0-9:+#]+(\-[a-z0-9:+#]+)*$/.test(value)
+    );
+  },
 };
 
 /**