Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@
"subscriptions-transport-ws": "^0.9.16",
"typesafe-actions": "^4.2.1",
"victory": "^37.3.6",
"yup": "^0.27.0"
"yup": "^1.7.1"
},
"devDependencies": {
"@babel/core": "^7.10.3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ export const containerImagePage = {
},
selectCustomIcon: (url: string) => {
cy.get(containerImagePO.imageSection.addCustomIcon).click();
// Wait for the modal to open and the URL input to be enabled (yup async validation)
cy.get(containerImagePO.imageSection.customIconModal.url)
.should('be.visible')
.and('not.be.disabled');
cy.get(containerImagePO.imageSection.customIconModal.url).type(url);
cy.get(containerImagePO.imageSection.customIconModal.confirmButton).click();
},
Expand Down
2 changes: 0 additions & 2 deletions frontend/packages/dev-console/locales/en/devconsole.json
Original file line number Diff line number Diff line change
Expand Up @@ -607,11 +607,9 @@
"Port must be between 1 and 65535.": "Port must be between 1 and 65535.",
"Request must be greater than or equal to 0.": "Request must be greater than or equal to 0.",
"CPU request must be less than or equal to limit.": "CPU request must be less than or equal to limit.",
"Unit must be millicores or cores.": "Unit must be millicores or cores.",
"Limit must be greater than or equal to 0.": "Limit must be greater than or equal to 0.",
"CPU limit must be greater than or equal to request.": "CPU limit must be greater than or equal to request.",
"Memory request must be less than or equal to limit.": "Memory request must be less than or equal to limit.",
"Unit must be Mi or Gi.": "Unit must be Mi or Gi.",
"Memory limit must be greater than or equal to request.": "Memory limit must be greater than or equal to request.",
"Please enter a URL that is less then 2000 characters.": "Please enter a URL that is less then 2000 characters.",
"Invalid Git URL.": "Invalid Git URL.",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,18 @@ const sourceSchema = () =>
.oneOf(['git', 'dockerfile', 'binary']),
git: yup.object().when('type', {
is: 'git',
then: yup.object({
git: yup.object({
url: yup.string().required(i18n.t('devconsole~Required')),
ref: yup.string(),
dir: yup.string(),
then: (schema) =>
schema.shape({
git: yup.object({
url: yup.string().required(i18n.t('devconsole~Required')),
ref: yup.string(),
dir: yup.string(),
}),
}),
}),
}),
dockerfile: yup.string().when('type', {
is: 'dockerfile',
then: yup.string(),
then: (schema) => schema,
}),
})
.required(i18n.t('devconsole~Required'));
Expand All @@ -34,21 +35,22 @@ const imageSchema = (allowedTypes: string[]) =>
type: yup.string().required(i18n.t('devconsole~Required')).oneOf(allowedTypes),
imageStreamTag: yup.object().when('type', {
is: 'imageStreamTag',
then: yup.object({
imageStream: yup.object({
namespace: yup.string().required(i18n.t('devconsole~Required')),
image: yup.string().required(i18n.t('devconsole~Required')),
tag: yup.string().required(i18n.t('devconsole~Required')),
then: (schema) =>
schema.shape({
imageStream: yup.object({
namespace: yup.string().required(i18n.t('devconsole~Required')),
image: yup.string().required(i18n.t('devconsole~Required')),
tag: yup.string().required(i18n.t('devconsole~Required')),
}),
}),
}),
}),
imageStreamImage: yup.string().when('type', {
is: 'imageStreamImage',
then: yup.string().required(i18n.t('devconsole~Required')),
then: (schema) => schema.required(i18n.t('devconsole~Required')),
}),
dockerImage: yup.string().when('type', {
is: 'dockerImage',
then: yup.string().required(i18n.t('devconsole~Required')),
then: (schema) => schema.required(i18n.t('devconsole~Required')),
}),
});

Expand Down Expand Up @@ -82,22 +84,23 @@ const formDataSchema = () =>

export const validationSchema = () =>
yup.mixed().test({
test(values: BuildConfigFormikValues) {
async test(values: BuildConfigFormikValues) {
const formYamlDefinition = yup.object({
editorType: yup
.string()
.oneOf(Object.values(EditorType))
.required(i18n.t('devconsole~Required')),
formData: yup.mixed().when('editorType', {
is: EditorType.Form,
then: formDataSchema(),
then: (schema) => schema.concat(formDataSchema()),
}),
yamlData: yup.mixed().when('editorType', {
is: EditorType.YAML,
then: yup.string().required(i18n.t('devconsole~Required')),
then: (schema) => schema.concat(yup.string().required(i18n.t('devconsole~Required'))),
}),
});

return formYamlDefinition.validate(values, { abortEarly: false });
await formYamlDefinition.validate(values, { abortEarly: false });
return true;
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -113,32 +113,35 @@ export const editDeploymentFormSchema = (formValues: EditDeploymentFormData) =>
: {}),
imageStream: yup.object().when('fromImageStreamTag', {
is: true,
then: yup.object({
namespace: yup.string().required(i18n.t('devconsole~Required')),
image: yup.string().required(i18n.t('devconsole~Required')),
tag: yup.string().required(i18n.t('devconsole~Required')),
}),
then: (schema) =>
schema.shape({
namespace: yup.string().required(i18n.t('devconsole~Required')),
image: yup.string().required(i18n.t('devconsole~Required')),
tag: yup.string().required(i18n.t('devconsole~Required')),
}),
}),
isi: yup.object().when('fromImageStreamTag', {
is: true,
then: yup.object({
name: yup.string().required(i18n.t('devconsole~Required')),
}),
then: (schema) =>
schema.shape({
name: yup.string().required(i18n.t('devconsole~Required')),
}),
}),
});

export const validationSchema = () =>
yup.mixed().test({
test(formValues: EditDeploymentData) {
async test(formValues: EditDeploymentData) {
const formYamlDefinition = yup.object({
editorType: yup.string().oneOf(Object.values(EditorType)),
yamlData: yup.string(),
formData: yup.mixed().when('editorType', {
is: EditorType.Form,
then: editDeploymentFormSchema(formValues.formData),
then: (schema) => schema.concat(editDeploymentFormSchema(formValues.formData)),
}),
});

return formYamlDefinition.validate(formValues, { abortEarly: false });
await formYamlDefinition.validate(formValues, { abortEarly: false });
return true;
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,55 +13,63 @@ export const healthChecksValidationSchema = (t: TFunction) =>
modified: yup.boolean(),
data: yup.object().when('showForm', {
is: true,
then: yup.object().shape({
periodSeconds: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(1, t('devconsole~Period must be greater than or equal to 1.'))
.max(MAX_INT32, t('devconsole~Value is larger than maximum value allowed.')),
initialDelaySeconds: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(0, t('devconsole~Initial delay must be greater than or equal to 0.'))
.max(MAX_INT32, t('devconsole~Value is larger than maximum value allowed.')),
failureThreshold: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(1, t('devconsole~Failure threshold must be greater than or equal to 1.')),
timeoutSeconds: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(1, t('devconsole~Timeout must be greater than or equal to 1.'))
.max(MAX_INT32, t('devconsole~Value is larger than maximum value allowed.')),
successThreshold: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(1, t('devconsole~Success threshold must be greater than or equal to 1.'))
.max(MAX_INT32, t('devconsole~Value is larger than maximum value allowed.')),
requestType: yup.string(),
httpGet: yup.object().when('requestType', {
is: 'httpGet',
then: yup.object({
path: yup.string().matches(pathRegex, {
message: t('devconsole~Path must start with /.'),
excludeEmptyString: true,
}),
port: yup.number().required(t('devconsole~Required')),
then: (schema) =>
schema.shape({
periodSeconds: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(1, t('devconsole~Period must be greater than or equal to 1.'))
.max(MAX_INT32, t('devconsole~Value is larger than maximum value allowed.')),
initialDelaySeconds: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(0, t('devconsole~Initial delay must be greater than or equal to 0.'))
.max(MAX_INT32, t('devconsole~Value is larger than maximum value allowed.')),
failureThreshold: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(1, t('devconsole~Failure threshold must be greater than or equal to 1.')),
timeoutSeconds: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(1, t('devconsole~Timeout must be greater than or equal to 1.'))
.max(MAX_INT32, t('devconsole~Value is larger than maximum value allowed.')),
successThreshold: yup
.number()
.integer(t('devconsole~Value must be an integer.'))
.min(1, t('devconsole~Success threshold must be greater than or equal to 1.'))
.max(MAX_INT32, t('devconsole~Value is larger than maximum value allowed.')),
requestType: yup.string(),
httpGet: yup.object().when('requestType', {
is: 'httpGet',
then: (httpGetSchema) =>
httpGetSchema.shape({
path: yup.string().matches(pathRegex, {
message: t('devconsole~Path must start with /.'),
excludeEmptyString: true,
}),
port: yup.number().required(t('devconsole~Required')),
}),
otherwise: (httpGetSchema) => httpGetSchema,
}),
}),
tcpSocket: yup.object().when('requestType', {
is: 'tcpSocket',
then: yup.object({
port: yup.number().required(t('devconsole~Required')),
tcpSocket: yup.object().when('requestType', {
is: 'tcpSocket',
then: (tcpSocketSchema) =>
tcpSocketSchema.shape({
port: yup.number().required(t('devconsole~Required')),
}),
otherwise: (tcpSocketSchema) => tcpSocketSchema,
}),
}),
exec: yup.object().when('requestType', {
is: 'command',
then: yup.object({
command: yup.array().of(yup.string().required(t('devconsole~Required'))),
exec: yup.object().when('requestType', {
is: 'command',
then: (execSchema) =>
execSchema.shape({
command: yup.array().of(yup.string().required(t('devconsole~Required'))),
}),
otherwise: (execSchema) => execSchema,
}),
}),
}),
otherwise: (schema) => schema,
}),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,49 +9,50 @@ export const hpaValidationSchema = (t: TFunction) =>
editorType: yup.string(),
formData: yup.object().when('editorType', {
is: EditorType.Form,
then: yup.object({
metadata: yup.object({
name: yup
.string()
.matches(nameRegex, {
message: t(
'devconsole~Name must consist of lower-case letters, numbers and hyphens. It must start with a letter and end with a letter or number.',
then: (schema) =>
schema.shape({
metadata: yup.object({
name: yup
.string()
.matches(nameRegex, {
message: t(
'devconsole~Name must consist of lower-case letters, numbers and hyphens. It must start with a letter and end with a letter or number.',
),
excludeEmptyString: true,
})
.max(253, t('devconsole~Cannot be longer than 253 characters.'))
.required(t('devconsole~Required')),
}),
spec: yup.object({
minReplicas: yup
.number()
.test(isInteger(t('devconsole~Minimum Pods must be an integer.')))
.min(1, t('devconsole~Minimum Pods must greater than or equal to 1.'))
.test(
'test-less-than-max',
t('devconsole~Minimum Pods should be less than or equal to Maximum Pods.'),
function (minReplicas) {
const { maxReplicas } = this.parent;
return minReplicas <= maxReplicas;
},
Comment on lines +27 to +37
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Make cross-field minReplicas/maxReplicas checks null-safe to avoid unintended validation failures

As written, the cross-field tests assume both minReplicas and maxReplicas are always present and that this.parent is never undefined:

  • If a user sets maxReplicas while minReplicas is still empty/undefined, the maxReplicas test will still run and evaluate minReplicas <= maxReplicas, which can fail purely because minReplicas is unset. That effectively makes minReplicas required even though the schema does not mark it as required.
  • The same pattern exists on the minReplicas side, and both tests destructure this.parent without a guard, which is fragile if the parent shape is ever absent or restructured.

To keep the UX predictable and avoid spurious cross-field errors, it’s safer to:

  • Skip the relational checks when either side is null/undefined.
  • Guard this.parent before destructuring.

Example patch:

           spec: yup.object({
             minReplicas: yup
               .number()
               .test(isInteger(t('devconsole~Minimum Pods must be an integer.')))
               .min(1, t('devconsole~Minimum Pods must greater than or equal to 1.'))
               .test(
                 'test-less-than-max',
                 t('devconsole~Minimum Pods should be less than or equal to Maximum Pods.'),
                 function (minReplicas) {
-                  const { maxReplicas } = this.parent;
-                  return minReplicas <= maxReplicas;
+                  const { maxReplicas } = this.parent || {};
+                  if (minReplicas == null || maxReplicas == null) {
+                    // Don’t enforce cross-field constraint until both values are set
+                    return true;
+                  }
+                  return minReplicas <= maxReplicas;
                 },
               ),
             maxReplicas: yup
               .number()
               .test(isInteger(t('devconsole~Maximum Pods must be an integer.')))
               .max(
                 Number.MAX_SAFE_INTEGER,
                 t('devconsole~Value is larger than maximum value allowed.'),
               )
               .test(
                 'test-greater-than-min',
                 t('devconsole~Maximum Pods should be greater than or equal to Minimum Pods.'),
                 function (maxReplicas) {
-                  const { minReplicas } = this.parent;
-                  return minReplicas <= maxReplicas;
+                  const { minReplicas } = this.parent || {};
+                  if (minReplicas == null || maxReplicas == null) {
+                    // Only validate the relationship when both sides are present
+                    return true;
+                  }
+                  return minReplicas <= maxReplicas;
                 },
               )
               .required(t('devconsole~Max Pods must be defined.')),
           }),

This keeps the intended invariant (minReplicas <= maxReplicas) while avoiding turning minReplicas into an implicit required field and makes the validation more robust to shape changes.

Also applies to: 39-54

🤖 Prompt for AI Agents
In frontend/packages/dev-console/src/components/hpa/validation-utils.ts around
lines 27-37 (and similarly 39-54), the cross-field tests for
minReplicas/maxReplicas assume this.parent exists and both values are present,
causing spurious failures when one side is undefined; update the test callbacks
to guard this.parent before destructuring and return true (skip the relational
check) if either minReplicas or maxReplicas is null/undefined so the
relationship check only runs when both values are set.

),
excludeEmptyString: true,
})
.max(253, t('devconsole~Cannot be longer than 253 characters.'))
.required(t('devconsole~Required')),
maxReplicas: yup
.number()
.test(isInteger(t('devconsole~Maximum Pods must be an integer.')))
.max(
Number.MAX_SAFE_INTEGER,
t('devconsole~Value is larger than maximum value allowed.'),
)
.test(
'test-greater-than-min',
t('devconsole~Maximum Pods should be greater than or equal to Minimum Pods.'),
function (maxReplicas) {
const { minReplicas } = this.parent;
return minReplicas <= maxReplicas;
},
)
.required(t('devconsole~Max Pods must be defined.')),
}),
}),
spec: yup.object({
minReplicas: yup
.number()
.test(isInteger(t('devconsole~Minimum Pods must be an integer.')))
.min(1, t('devconsole~Minimum Pods must greater than or equal to 1.'))
.test(
'test-less-than-max',
t('devconsole~Minimum Pods should be less than or equal to Maximum Pods.'),
function (minReplicas) {
const { maxReplicas } = this.parent;
return minReplicas <= maxReplicas;
},
),
maxReplicas: yup
.number()
.test(isInteger(t('devconsole~Maximum Pods must be an integer.')))
.max(
Number.MAX_SAFE_INTEGER,
t('devconsole~Value is larger than maximum value allowed.'),
)
.test(
'test-greater-than-min',
t('devconsole~Maximum Pods should be greater than or equal to Minimum Pods.'),
function (maxReplicas) {
const { minReplicas } = this.parent;
return minReplicas <= maxReplicas;
},
)
.required(t('devconsole~Max Pods must be defined.')),
}),
}),
}),
});
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ export const deployValidationSchema = (t: TFunction) =>
project: projectNameValidationSchema,
application: applicationNameValidationSchema,
name: nameValidationSchema(t),
isi: isiValidationSchema(t),
isi: yup.object().when('registry', {
is: 'internal',
then: (schema) => schema.concat(isiValidationSchema(t)),
otherwise: (schema) => schema,
}),
serverless: serverlessValidationSchema(t),
deployment: deploymentValidationSchema(t),
route: routeValidationSchema(t),
Expand Down
Loading