Skip to content
This repository has been archived by the owner on Apr 28, 2020. It is now read-only.

Align vmware secret with DNS 1123 #549

Merged
merged 1 commit into from
Sep 11, 2019
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const getVmWareProviderRequestedResources = state => {
namespace: VMWARE_TO_KUBEVIRT_OS_CONFIG_MAP_NAMESPACE,
isList: false,
prop: 'vmwareToKubevirtOsConfigMap',
optional: true,
}),
];

Expand Down
2 changes: 1 addition & 1 deletion src/k8s/requests/v2v/startV2VvmwareController.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const startV2VVMWareController = async ({ namespace }, { k8sGet, k8sCreat
}
semaphors[namespace] = true;

const kubevirtVmwareConfigMap = getVmwareConfigMap({ k8sGet });
const kubevirtVmwareConfigMap = await getVmwareConfigMap({ k8sGet });

try {
if (!kubevirtVmwareConfigMap) {
Expand Down
6 changes: 5 additions & 1 deletion src/k8s/requests/v2v/tests/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,16 @@ describe('v2v requests', () => {
const usernameEmpty = '';
const usernameDomain = 'mydomainuser@domain.com';
const usernameLong = 'verylongusernameexceedinglimits@very-long-domain-which-makes-no-sense.com';
const usernameNonalphanum = '.nonalpha@.-';
const usernameNonalphanum2 = '.@.-';

expect(getDefaultSecretName({ username, url })).toEqual('myuser-my.host.com');
expect(getDefaultSecretName({ username, url: urlNoProtocol })).toEqual('myuser-my.host.com');
expect(getDefaultSecretName({ username, url: urlSingleDomain })).toEqual('myuser-my-host');
expect(getDefaultSecretName({ username, url: urlLong })).toEqual('myuser-my-host.with-very-long-');
expect(getDefaultSecretName({ username, url: urlLong })).toEqual('myuser-my-host.with-very-long');

expect(getDefaultSecretName({ username: usernameNonalphanum, url })).toEqual('nonalpha-my.host.com');
expect(getDefaultSecretName({ username: usernameNonalphanum2, url })).toEqual('nouser-my.host.com');
expect(getDefaultSecretName({ username: usernameNone, url })).toEqual('nousername-my.host.com');
expect(getDefaultSecretName({ username: usernameEmpty, url })).toEqual('nousername-my.host.com');
expect(getDefaultSecretName({ username: usernameDomain, url })).toEqual('mydomainuser-my.host.com');
Expand Down
30 changes: 28 additions & 2 deletions src/k8s/requests/v2v/utils.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,30 @@
import { alphanumericRegex } from '../../../utils';

/**
* Based on V2V Provider Pod manifest.yaml
*/

const MAX_LEN = 30;

// TODO: this needs to be improved to conform validations.js::validateDNS1123SubdomainValue()
const alignWithDNS1123 = str => {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we rename it to trimAccordingToDNS1123 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My intention is to further improve this method to fully conform DNS 1123, not just start and end. This first version was meant as a quick fix to unblock testing immediately. So it makes sense to keep the name as it is.

if (!str) {
return '';
}

// starts with alphaNum
if (!str.charAt(0).match(alphanumericRegex)) {
return alignWithDNS1123(str.slice(1));
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be done much easier without a recursion

str.replace(/^[^a-zA-Z0-9]*/, '').replace(/[^a-zA-Z0-9]*$/, '')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, but I like recursion :-)

I can change it.

}

// ends with alphaNum
if (!str.charAt(str.length - 1).match(alphanumericRegex)) {
return alignWithDNS1123(str.slice(0, -1));
}

return str;
};

export const getDefaultSecretName = ({ username, url }) => {
if (!url) {
throw new Error('VMware URL can not be empty.');
Expand All @@ -15,8 +36,13 @@ export const getDefaultSecretName = ({ username, url }) => {
const u = new URL(url);
username = username || 'nousername';

const user = username.split('@')[0].substring(0, 15);
const host = (u.host || 'nohost').substring(0, MAX_LEN - user.length - 1);
let user = username.split('@')[0].substring(0, 15);
let host = (u.host || 'nohost').substring(0, MAX_LEN - user.length - 1);

user = alignWithDNS1123(user);
host = alignWithDNS1123(host);

user = user || 'nouser';

return `${user}-${host}`;
};
2 changes: 1 addition & 1 deletion src/utils/validations.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { joinGrammaticallyListOfItems, makeSentence } from './grammar';

export const isPositiveNumber = value => value && value.toString().match(/^[1-9]\d*$/);

const alphanumericRegex = '[a-zA-Z0-9]';
export const alphanumericRegex = '[a-zA-Z0-9]';

const DNS_1123_OFFENDING_CHARACTERS = {
',': 'comma',
Expand Down