Skip to content

14144 s3 endpoint field in cluster configuration missing input validation #14557

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.

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

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
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
1 change: 1 addition & 0 deletions shell/assets/translations/en-us.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,7 @@ cluster:
defaultEndpoint:
label: Default Endpoint
placeholder: "Optional: The default endpoint to use"
error: Endpoint URL cannot start with protocol (e.g. https://, http://)
defaultFolder:
label: Default Folder
placeholder: "Optional: The default folder to use"
Expand Down
66 changes: 57 additions & 9 deletions shell/edit/provisioning.cattle.io.cluster/rke2.vue
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ import ClusterAppearance from '@shell/components/form/ClusterAppearance';
import AddOnAdditionalManifest from '@shell/edit/provisioning.cattle.io.cluster/tabs/AddOnAdditionalManifest';
import VsphereUtils, { VMWARE_VSPHERE } from '@shell/utils/v-sphere';
import { mapGetters } from 'vuex';
import { isHttpsOrHttp } from '@shell/utils/validators/setting';
import S3Config from '@shell/edit/provisioning.cattle.io.cluster/tabs/etcd/S3Config.vue';
const HARVESTER = 'harvester';
const HARVESTER_CLOUD_PROVIDER = 'harvester-cloud-provider';
const NETBIOS_TRUNCATION_LENGTH = 15;
Expand Down Expand Up @@ -118,7 +120,8 @@ export default {
AddOnConfig,
Advanced,
ClusterAppearance,
AddOnAdditionalManifest
AddOnAdditionalManifest,
S3Config,
},

mixins: [CreateEditView, FormValidation],
Expand All @@ -142,7 +145,13 @@ export default {
providerConfig: {
type: Object,
default: () => null
}
},

s3EndpointHasError: {
type: Boolean,
default: false,
},

},

async fetch() {
Expand Down Expand Up @@ -172,6 +181,12 @@ export default {
this.value.spec.rkeConfig = {};
}

if (!this.value.spec.rkeConfig.etcd) {
this.value.spec.rkeConfig.etcd = {};
}

const initialS3Config = this.value.spec.rkeConfig.etcd?.s3 || {};

if (!this.value.spec.rkeConfig.chartValues) {
this.value.spec.rkeConfig.chartValues = {};
}
Expand Down Expand Up @@ -266,13 +281,28 @@ export default {
clusterAgentDefaultPDB: null,
activeTab: null,
REGISTRIES_TAB_NAME,
labelForAddon

labelForAddon,
s3ConfigValue: { ...initialS3Config },
};
},

computed: {
...mapGetters({ features: 'features/get' }),

isS3EndpointTrulyValid() {
const s3EndpointValue = this.rkeConfig.etcd?.s3?.endpoint;

this.s3ConfigValue.endpoint = s3EndpointValue || '';
// if empty or is not protocol
if (isEmpty(s3EndpointValue) || !isHttpsOrHttp(s3EndpointValue)) {
return true;
}
if (this.s3ConfigValue) {
return this.s3EndpointHasError;
}

return false;
},
isActiveTabRegistries() {
return this.activeTab?.selectedName === REGISTRIES_TAB_NAME;
},
Expand Down Expand Up @@ -851,7 +881,12 @@ export default {
set(newValue) {
this.$emit('update:value', newValue);
}
}
},
overallFormValidationPassed() {
return this.validationPassed &&
this.fvFormIsValid &&
this.isS3EndpointTrulyValid;
},
},

watch: {
Expand Down Expand Up @@ -2051,8 +2086,10 @@ export default {
if (isEmpty(this.rkeConfig.etcd?.s3)) {
this.rkeConfig.etcd.s3 = {};
}
this.s3ConfigValue = this.rkeConfig.etcd.s3;
} else {
this.rkeConfig.etcd.s3 = null;
this.s3ConfigValue = {};
}
},
handleConfigEtcdExposeMetricsChanged(neu) {
Expand Down Expand Up @@ -2118,7 +2155,7 @@ export default {
v-else
ref="cruresource"
:mode="mode"
:validation-passed="validationPassed && fvFormIsValid"
:validation-passed="overallFormValidationPassed"
:resource="value"
:errors="errors"
:cancel-event="true"
Expand Down Expand Up @@ -2337,12 +2374,23 @@ export default {
:s3-backup="s3Backup"
:register-before-hook="registerBeforeHook"
:selected-version="selectedVersion"
@update:value="$emit('input', $event)"
@s3-backup-changed="handleS3BackupChanged"
@config-etcd-expose-metrics-changed="handleConfigEtcdExposeMetricsChanged"
/>
@update:value="$emit('input', $event)"
>
<template #s3-config>
<S3Config
v-show="s3Backup"
ref="s3ConfigComponent"
v-model:value="s3ConfigValue"
:mode="mode"
:namespace="value.metadata.namespace"
:register-before-hook="registerBeforeHook"
@update:value="$emit('input', $event)"
/>
</template>
</Etcd>
</Tab>

<!-- Networking -->
<Tab
v-if="haveArgInfo"
Expand Down
58 changes: 57 additions & 1 deletion shell/edit/provisioning.cattle.io.cluster/tabs/etcd/S3Config.vue
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { Checkbox } from '@components/Form/Checkbox';
import { LabeledInput } from '@components/Form/LabeledInput';
import SelectOrCreateAuthSecret from '@shell/components/form/SelectOrCreateAuthSecret';
import { NORMAN } from '@shell/config/types';
import FormValidation from '@shell/mixins/form-validation';
import { isHttpsOrHttp } from '@shell/utils/validators/setting';

export default {
emits: ['update:value'],
Expand All @@ -12,6 +14,7 @@ export default {
Checkbox,
SelectOrCreateAuthSecret,
},
mixins: [FormValidation],

props: {
mode: {
Expand Down Expand Up @@ -48,10 +51,14 @@ export default {
...(this.value || {}),
};

return { config };
return {
config,
s3EndpointHasError: false,
};
},

computed: {

ccData() {
if ( this.config.cloudCredentialName ) {
const cred = this.$store.getters['rancher/byId'](NORMAN.CLOUD_CREDENTIAL, this.config.cloudCredentialName);
Expand All @@ -63,22 +70,66 @@ export default {

return {};
},

isEndpointInvalid() {
return this.s3EndpointHasError;
}
},

methods: {
update() {
const out = { ...this.config };

this.$emit('update:value', out);

this.validateEndpoint(this.config.endpoint);
},
isHttpsOrHttp,

validateEndpoint(value) {
let message = '';

if (isHttpsOrHttp(this.value.endpoint)) {
message = this.t('cluster.credential.s3.defaultEndpoint.error');
this.s3EndpointHasError = !!message; // Set to true if a message exists, false otherwise
} else {
this.s3EndpointHasError = false;
}

return this.s3EndpointHasError;
},
endpointMessage() {
return this.s3EndpointHasError ? this.t('cluster.credential.s3.defaultEndpoint.error') : null;
},
},

watch: {
'config.endpoint': {
handler(newValue) {
this.validateEndpoint(newValue);
},
immediate: true,
},
value: {
handler(newValue) {
if (newValue?.endpoint !== this.config.endpoint) {
this.config.endpoint = newValue?.endpoint || '';
}
this.validateEndpoint(this.config.endpoint);
},
deep: true,
immediate: true,
}
},

};
</script>

<template>
<div>
<SelectOrCreateAuthSecret
v-model:value="config.cloudCredentialName"
:mode="mode"
:register-before-hook="registerBeforeHook"
in-store="management"
:allow-ssh="false"
Expand All @@ -95,6 +146,7 @@ export default {
<LabeledInput
v-model:value="config.bucket"
label="Bucket"
:mode="mode"
:placeholder="ccData.defaultBucket"
:required="!ccData.defaultBucket"
@update:value="update"
Expand All @@ -104,6 +156,7 @@ export default {
<LabeledInput
v-model:value="config.folder"
label="Folder"
:mode="mode"
:placeholder="ccData.defaultFolder"
@update:value="update"
/>
Expand All @@ -115,6 +168,7 @@ export default {
<LabeledInput
v-model:value="config.region"
label="Region"
:mode="mode"
:placeholder="ccData.defaultRegion"
@update:value="update"
/>
Expand All @@ -123,7 +177,9 @@ export default {
<LabeledInput
v-model:value="config.endpoint"
label="Endpoint"
:mode="mode"
:placeholder="ccData.defaultEndpoint"
:rules="[endpointMessage]"
@update:value="update"
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export default {
configEtcdExposeMetrics() {
return !!this.value.spec.rkeConfig.machineGlobalConfig['etcd-expose-metrics'];
},

},
};
</script>
Expand Down
5 changes: 4 additions & 1 deletion shell/utils/validators/formRules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import has from 'lodash/has';
import isUrl from 'is-url';
// import uniq from 'lodash/uniq';
import { Translation } from '@shell/types/t';
import { isHttps, isLocalhost, hasTrailingForwardSlash } from '@shell/utils/validators/setting';
import { isHttps, isLocalhost, hasTrailingForwardSlash, isHttpsOrHttp } from '@shell/utils/validators/setting';
import { cronScheduleRule } from '@shell/utils/validators/cron-schedule';

// import uniq from 'lodash/uniq';
Expand Down Expand Up @@ -164,6 +164,8 @@ export default function(

const https: Validator = (val: string) => val && !isHttps(val) ? t('validation.setting.serverUrl.https') : undefined;

const httpsOrHttp: Validator = (val: string) => val && !isHttpsOrHttp(val) ? t('validation.setting.serverUrl.https') : undefined;

const localhost: Validator = (val: string) => isLocalhost(val) ? t('validation.setting.serverUrl.localhost') : undefined;

const trailingForwardSlash: Validator = (val: string) => hasTrailingForwardSlash(val) ? t('validation.setting.serverUrl.trailingForwardSlash') : undefined;
Expand Down Expand Up @@ -533,6 +535,7 @@ export default function(
imageUrl,
interval,
https,
httpsOrHttp,
localhost,
trailingForwardSlash,
url,
Expand Down
10 changes: 10 additions & 0 deletions shell/utils/validators/setting.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ export const isServerUrl = (value) => value === 'server-url';

export const isHttps = (value) => value.toLowerCase().startsWith('https://');

export const isHttpsOrHttp = (value) => {
if (typeof value !== 'string') {
return false; // Or throw new Error('Input is not a string');
}

const lowerCaseValue = value.toLowerCase();

return lowerCaseValue.startsWith('https://') || lowerCaseValue.startsWith('http://');
};

export const isLocalhost = (value) => (/^(?:https?:\/\/)?(?:localhost|127\.0\.0\.1)/i).test(value);

export const hasTrailingForwardSlash = (value) => isUrl(value) && value?.toLowerCase().endsWith('/');