Skip to content

Commit

Permalink
Merge pull request #70 from Kuadrant/validation
Browse files Browse the repository at this point in the history 
Fix showing of server side validation error response in RLP & DNS cre…
  • Loading branch information
@openshift-merge-bot
openshift-merge-bot[bot] authored Sep 17, 2024
2 parents 1523aa9 + 9cfea08 commit 17a5d6f
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 153 deletions.
3 changes: 2 additions & 1 deletion locales/en/plugin__console-plugin-template.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,5 +68,6 @@
"Configured Limits": "Configured Limits",
"Target reference type": "Target reference type",
"Unique name of the RateLimitPolicy": "Unique name of the RateLimitPolicy",
"RateLimitPolicy enables rate limiting for service workloads in a Gateway API network": "RateLimitPolicy enables rate limiting for service workloads in a Gateway API network"
"RateLimitPolicy enables rate limiting for service workloads in a Gateway API network": "RateLimitPolicy enables rate limiting for service workloads in a Gateway API network",
"To set defaults, overrides, and more complex limits, use the YAML view.": "To set defaults, overrides, and more complex limits, use the YAML view."
}
1 change: 0 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@
"babel-loader": "^8.2.0",
"graphlib": "^2.1.8",
"graphlib-dot": "^0.6.4",
"js-yaml": "^4.1.0",
"react-policy-topology": "^0.1.10"
}
}
89 changes: 27 additions & 62 deletions src/components/KuadrantDNSPolicyCreatePage.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as React from 'react';
import Helmet from 'react-helmet';
import yaml from 'js-yaml';
import {
PageSection,
Title,
Expand All @@ -14,8 +13,10 @@ import {
Button,
ExpandableSection,
ButtonVariant,
Modal,
ActionGroup,
AlertVariant,
Alert,
AlertGroup,
} from '@patternfly/react-core';
import { useTranslation } from 'react-i18next';
import './kuadrant.css';
Expand All @@ -27,9 +28,8 @@ import HealthCheckField from './dnspolicy/HealthCheckField';
import getModelFromResource from '../utils/getModelFromResource';
import { Gateway } from './gateway/types';
import GatewaySelect from './gateway/GatewaySelect';
import { ModalHeader, ModalBody, ModalFooter } from '@patternfly/react-core/next';
import NamespaceSelect from './namespace/NamespaceSelect';
import { removeUndefinedFields, convertMatchLabelsArrayToObject, convertMatchLabelsObjectToArray } from '../utils/modelUtils';
import { removeUndefinedFields, convertMatchLabelsArrayToObject } from '../utils/modelUtils';

const KuadrantDNSPolicyCreatePage: React.FC = () => {
const { t } = useTranslation('plugin__console-plugin-template');
Expand All @@ -48,6 +48,7 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => {
port: null,
protocol: 'HTTP',
});
const [isCreateButtonDisabled, setIsCreateButtonDisabled] = React.useState(true);

// Initialize the YAML resource object based on form state
const [yamlResource, setYamlResource] = React.useState(() => {
Expand Down Expand Up @@ -107,12 +108,15 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => {
healthCheck: healthCheck.endpoint ? healthCheck : undefined,
},
};

setYamlResource(removeUndefinedFields(updatedYamlResource)); // Clean undefined values
}, [policy, selectedNamespace, selectedGateway, routingStrategy, loadBalancing, healthCheck]);

const [isErrorModalOpen, setIsErrorModalOpen] = React.useState(false);
const [errorModalMsg, setErrorModalMsg] = React.useState('')
// Check if the Create button should be enabled
const isFormValid = policy && selectedNamespace && selectedGateway.name;
setIsCreateButtonDisabled(!isFormValid); // Update the button state
}, [policy, selectedNamespace, selectedGateway, routingStrategy, loadBalancing, healthCheck]);

const [errorAlertMsg, setErrorAlertMsg] = React.useState('')

const handleCreateViewChange = (value: 'form' | 'yaml') => {
setCreateView(value);
Expand All @@ -122,39 +126,10 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => {
setPolicy(policy);
};

const handleYamlSave = (content: string) => {
try {
const parsedYaml = yaml.load(content) as DNSPolicy;
if (parsedYaml) {
setPolicy(parsedYaml.metadata.name || '');
setSelectedNamespace(parsedYaml.metadata.namespace || '');
setRoutingStrategy(parsedYaml.spec.routingStrategy);
setSelectedGateway(parsedYaml.spec.targetRef);
setLoadBalancing({
...parsedYaml.spec.loadBalancing,
weighted: {
...parsedYaml.spec.loadBalancing?.weighted,
custom: parsedYaml.spec.loadBalancing?.weighted?.custom?.map((customWeight) => ({
...customWeight,
selector: {
...customWeight.selector,
// Convert map back to array for form rendering
matchLabels: convertMatchLabelsObjectToArray(
(typeof customWeight.selector.matchLabels === 'object' ? customWeight.selector.matchLabels : {}) as { [key: string]: string }
),
},
})),
},
});
setHealthCheck(parsedYaml.spec.healthCheck || healthCheck);
}
} catch (error) {
console.error('Error parsing YAML:', error);
}
handleSubmit();
};

const handleSubmit = async () => {
if (isCreateButtonDisabled) return; // Early return if form is not valid
setErrorAlertMsg('')

const isHealthCheckValid =
healthCheck.endpoint &&
healthCheck.failureThreshold > 0 &&
Expand Down Expand Up @@ -216,9 +191,8 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => {
});
history.push('/kuadrant/all-namespaces/policies/dns'); // Navigate after successful creation
} catch (error) {
console.error(t('Error creating DNSPolicy'), error);
setErrorModalMsg(error)
setIsErrorModalOpen(true)
console.error(t('Error creating DNSPolicy'), { error });
setErrorAlertMsg(error.message)
}
};

Expand Down Expand Up @@ -290,8 +264,16 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => {
</ExpandableSection>
</div>

{errorAlertMsg != '' && (
<AlertGroup className="kuadrant-alert-group">
<Alert title={t('Error creating DNSPolicy')} variant={AlertVariant.danger} isInline>
{errorAlertMsg}
</Alert>
</AlertGroup>
)}

<ActionGroup>
<Button variant={ButtonVariant.primary} onClick={handleSubmit}>
<Button variant={ButtonVariant.primary} onClick={handleSubmit} isDisabled={isCreateButtonDisabled}>
{t('Create DNSPolicy')}
</Button>
<Button variant={ButtonVariant.secondary} onClick={handleCancel}>
Expand All @@ -301,25 +283,8 @@ const KuadrantDNSPolicyCreatePage: React.FC = () => {
</Form>
</PageSection>
) : (
<ResourceYAMLEditor initialResource={yamlResource} onSave={handleYamlSave} create />
<ResourceYAMLEditor initialResource={yamlResource} create />
)}
<Modal
isOpen={isErrorModalOpen}
onClose={() => setIsErrorModalOpen(false)}
aria-labelledby="error-modal-title"
aria-describedby="error-modal-body"
variant="medium"
>
<ModalHeader title={t('Error creating DNSPolicy')} />
<ModalBody>
<b>{errorModalMsg}</b>
</ModalBody>
<ModalFooter>
<Button key="ok" variant={ButtonVariant.link} onClick={() => setIsErrorModalOpen(false)}>
OK
</Button>
</ModalFooter>
</Modal>
</>
);
};
Expand Down
108 changes: 48 additions & 60 deletions src/components/KuadrantRateLimitPolicyCreatePage.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
Flex,
FlexItem,
Alert,
AlertGroup,
AlertVariant,
} from '@patternfly/react-core';
import { useTranslation } from 'react-i18next';
import './kuadrant.css';
Expand All @@ -28,7 +30,6 @@ import { LimitConfig, RateLimitPolicy } from './ratelimitpolicy/types'
import getModelFromResource from '../utils/getModelFromResource';
import { Gateway } from './gateway/types';
import GatewaySelect from './gateway/GatewaySelect';
import { ModalHeader, ModalBody, ModalFooter } from '@patternfly/react-core/next';
import NamespaceSelect from './namespace/NamespaceSelect';
import HTTPRouteSelect from './httproute/HTTPRouteSelect';
import { HTTPRoute } from './httproute/types';
Expand All @@ -54,6 +55,7 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => {
rates: [{ duration: 60, limit: 100, unit: 'minute' }]
});
const [rateName, setRateName] = React.useState<string>('');
const [isCreateButtonDisabled, setIsCreateButtonDisabled] = React.useState(true);

const handleOpenModal = () => {
// Reset temporary state when opening modal for new entry
Expand Down Expand Up @@ -110,30 +112,6 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => {
};
});

React.useEffect(() => {
const newTargetRef = targetType === 'gateway'
? {
group: 'gateway.networking.k8s.io',
kind: 'Gateway',
name: selectedGateway.name,
namespace: selectedGateway.namespace,
}
: {
group: 'gateway.networking.k8s.io',
kind: 'HTTPRoute',
name: selectedHTTPRoute.name,
namespace: selectedHTTPRoute.namespace,
};
setYamlResource((prevResource) => ({
...prevResource,
spec: {
...prevResource.spec,
targetRef: newTargetRef,
limits,
},
}));
}, [targetType, selectedGateway, selectedHTTPRoute]);

const history = useHistory();

React.useEffect(() => {
Expand All @@ -145,21 +123,31 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => {
namespace: selectedNamespace,
},
spec: {
targetRef: {
targetRef: targetType === 'gateway'
? {
group: 'gateway.networking.k8s.io',
kind: 'Gateway',
name: selectedGateway.name,
namespace: selectedGateway.namespace,
}
: {
group: 'gateway.networking.k8s.io',
kind: 'HTTPRoute',
name: selectedHTTPRoute.name,
namespace: selectedHTTPRoute.namespace,
},
limits: { ...limits },
},
};

setYamlResource(updatedYamlResource);
}, [policy, selectedNamespace, selectedGateway, limits]);

const [isErrorModalOpen, setIsErrorModalOpen] = React.useState(false);
const [errorModalMsg, setErrorModalMsg] = React.useState('')
// Check if the Create button should be enabled
const isFormValid = policy && selectedNamespace && (selectedGateway.name || selectedHTTPRoute.name);
setIsCreateButtonDisabled(!isFormValid); // Update the button state
}, [policy, selectedNamespace, targetType, selectedGateway, selectedHTTPRoute, limits]);

const [errorAlertMsg, setErrorAlertMsg] = React.useState('')

const handleCreateViewChange = (value: 'form' | 'yaml') => {
setCreateView(value);
Expand All @@ -170,6 +158,8 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => {
};

const handleSubmit = async () => {
if (isCreateButtonDisabled) return; // Early return if form is not valid
setErrorAlertMsg('')
const ratelimitPolicy: RateLimitPolicy = {
apiVersion: 'kuadrant.io/v1beta2',
kind: 'RateLimitPolicy',
Expand All @@ -178,12 +168,19 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => {
namespace: selectedNamespace,
},
spec: {
targetRef: {
group: 'gateway.networking.k8s.io',
kind: 'Gateway',
name: selectedGateway.name,
namespace: selectedGateway.namespace
},
targetRef: targetType === 'gateway'
? {
group: 'gateway.networking.k8s.io',
kind: 'Gateway',
name: selectedGateway.name,
namespace: selectedGateway.namespace,
}
: {
group: 'gateway.networking.k8s.io',
kind: 'HTTPRoute',
name: selectedHTTPRoute.name,
namespace: selectedHTTPRoute.namespace,
},
},
};

Expand All @@ -196,8 +193,7 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => {
history.push('/kuadrant/all-namespaces/policies/ratelimit'); // Navigate after successful creation
} catch (error) {
console.error(t('Error creating RateLimitPolicy'), error);
setErrorModalMsg(error)
setIsErrorModalOpen(true)
setErrorAlertMsg(error.message)
}
};

Expand Down Expand Up @@ -316,13 +312,22 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => {
</Button>
</Modal>
</div>
<Alert
variant="info"
isInline
title="To set defaults, overrides, and more complex limits, use the YAML view."
/>
<AlertGroup className="kuadrant-alert-group">
<Alert
variant="info"
isInline
title={t('To set defaults, overrides, and more complex limits, use the YAML view.')}
/>

{errorAlertMsg != '' && (
<Alert title={t('Error creating RateLimitPolicy')} variant={AlertVariant.danger} isInline>
{errorAlertMsg}
</Alert>
)}
</AlertGroup>

<ActionGroup>
<Button variant={ButtonVariant.primary} onClick={handleSubmit}>
<Button variant={ButtonVariant.primary} onClick={handleSubmit} isDisabled={isCreateButtonDisabled}>
{t('Create RateLimitPolicy')}
</Button>
<Button variant={ButtonVariant.secondary} onClick={handleCancel}>
Expand All @@ -334,23 +339,6 @@ const KuadrantRateLimitPolicyCreatePage: React.FC = () => {
) : (
<ResourceYAMLEditor initialResource={yamlResource} create />
)}
<Modal
isOpen={isErrorModalOpen}
onClose={() => setIsErrorModalOpen(false)}
aria-labelledby="error-modal-title"
aria-describedby="error-modal-body"
variant="medium"
>
<ModalHeader title={t('Error creating RateLimitPolicy')} />
<ModalBody>
<b>{errorModalMsg}</b>
</ModalBody>
<ModalFooter>
<Button key="ok" variant={ButtonVariant.link} onClick={() => setIsErrorModalOpen(false)}>
{t('OK')}
</Button>
</ModalFooter>
</Modal>
</>
);
};
Expand Down
2 changes: 1 addition & 1 deletion src/components/namespace/NamespaceSelect.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const NamespaceSelect: React.FC<NamespaceSelectProps> = ({ selectedNamespace, on
onChange(event.currentTarget.value);
};
return (
<FormGroup label={t('Namespace')} fieldId="namespace-select">
<FormGroup label={t('Namespace')} fieldId="namespace-select" isRequired>
<FormSelect
id="namespace-select"
value={selectedNamespace}
Expand Down
Loading

0 comments on commit 17a5d6f

Please sign in to comment.