Skip to content
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

Removed unwanted properties from steve resources #10037

Merged
merged 31 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
9ac7eb2
Removed unwanted properties from pod, secret and workload
Shavindra Nov 2, 2023
ae801b4
Fix tests.
Shavindra Nov 14, 2023
3436e42
Fix linting errors.
Shavindra Nov 14, 2023
eaafc3a
Fix imports.
Shavindra Nov 14, 2023
cbf36cc
Update import
Shavindra Nov 14, 2023
d3c6562
Fix imports
Shavindra Nov 14, 2023
de550a8
Merge branch 'master' into 9371-remove-unwatned-properties
Shavindra Dec 14, 2023
b750d77
Remove type.
Shavindra Dec 14, 2023
2292202
Fix linting.
Shavindra Dec 15, 2023
f7deebb
Fix e2e.
Shavindra Dec 15, 2023
3cd2148
Merge branch 'master' into 9371-remove-unwatned-properties
Shavindra Dec 15, 2023
f4ad064
remove type from test.
Shavindra Dec 15, 2023
40b323b
Remove type from request.
Shavindra Dec 15, 2023
d60eec8
Fix tests.
Shavindra Dec 15, 2023
47dcfda
Import just one function over the whole lodash utilis
cnotv Dec 15, 2023
03dba9f
Add test for resource class
cnotv Dec 15, 2023
bd11bde
Add tests fir steve class model on save
cnotv Dec 15, 2023
939e3e3
Add tests for workload save
cnotv Dec 15, 2023
6bf041f
Restore steve method fix deleting keys
cnotv Dec 18, 2023
b816b42
Remove type from resource
cnotv Dec 18, 2023
d1a8b2f
Refactoring and many fixes
richard-cox Dec 20, 2023
afa5feb
fix workload test
richard-cox Dec 20, 2023
aacf71b
fix resource-class test
richard-cox Dec 20, 2023
aa64618
improve unit tests, ensure super.cleanForSave's run correctly
richard-cox Dec 20, 2023
cc9136b
Fix unit tests (move mocks into utils folder)
richard-cox Jan 2, 2024
aaba6b3
Include metadata clusterName, deletionPeriodSeconds and generateName …
richard-cox Jan 2, 2024
3836bcc
fix / update fleet git repo e2e test
richard-cox Jan 2, 2024
f4be4c6
Fix cluster e2e test (and fleet git repo imports)
richard-cox Jan 2, 2024
acc9d67
Make workload pod e2e tests more patient
richard-cox Jan 2, 2024
83f0cd5
More hardening of pod spec test
richard-cox Jan 2, 2024
24a44da
Minor tweaks
richard-cox Jan 2, 2024
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 .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"eslint.packageManager": "yarn",
"eslint.validate": ["vue","html","javascript","typescript"],
"editor.codeActionsOnSave": {
"source.fixAll.eslint": true
"source.fixAll.eslint": "explicit"
richard-cox marked this conversation as resolved.
Show resolved Hide resolved
},
"eslint.workingDirectories": ["./", "./pkg/rancher-components/"],
"javascript.preferences.importModuleSpecifier": "non-relative",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export const createDeploymentBlueprint = {
};

export const deploymentCreateRequest = {
type: 'apps.deployment',
metadata: {
namespace: 'default',
labels: { 'workload.user.cattle.io/workloadselector': 'apps.deployment-default-test-deployment' },
Expand All @@ -58,9 +57,7 @@ export const deploymentCreateRequest = {
privileged: false,
allowPrivilegeEscalation: false
},
_init: false,
volumeMounts: [],
__active: true,
image: 'nginx'
}
],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
export const deploymentGetResponse = {
id: 'default/test-deployment',
type: 'apps.deployment',
links: {
remove: 'https://localhost:8005/v1/apps.deployments/default/test-deployment',
self: 'https://localhost:8005/v1/apps.deployments/default/test-deployment',
Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/blueprints/fleet/gitrepos.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export const gitRepoCreateRequest = {
type: 'fleet.cattle.io.gitrepo',
// type: 'fleet.cattle.io.gitrepo',
metadata: {
namespace: 'fleet-default',
name: 'fleet-e2e-test-gitrepo'
Expand Down
3 changes: 1 addition & 2 deletions cypress/e2e/tests/pages/manager/cluster-manager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('Cluster Manager', { testIsolation: 'off', tags: ['@manager', '@adminUs
it('can create new cluster', () => {
cy.intercept('POST', `/v1/${ type }s`).as('createRequest');
const request = {
type,
// type,
metadata: {
namespace,
name: rke2CustomName
Expand Down Expand Up @@ -270,7 +270,6 @@ describe('Cluster Manager', { testIsolation: 'off', tags: ['@manager', '@adminUs

cy.wait('@importRequest').then((intercept) => {
expect(intercept.request.body).to.deep.equal({
type,
metadata: {
namespace,
name: importGenericName
Expand Down
60 changes: 60 additions & 0 deletions shell/models/__tests__/workload.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import Workload from '@shell/models/workload.js';

describe('class: Workload', () => {
describe('given custom workload keys', () => {
const customContainer = {
__clone: 'whatever',
__active: 'whatever',
__init: 'whatever',
_init: 'whatever',
error: 'whatever',
};
const customWorkload = {
__rehydrate: 'whatever',
__clone: 'whatever',
spec: {
template: {
spec: {
containers: [customContainer],
initContainers: [customContainer],
}
}
}
};

it('should keep internal keys', () => {
const workload = new Workload(customWorkload, {
getters: { schemaFor: () => ({ linkFor: jest.fn() }) },
dispatch: jest.fn(),
rootGetters: { 'i18n/t': jest.fn() },
});

expect({ ...workload }).toStrictEqual(customWorkload);
});

describe('method: save', () => {
/**
* DISCLAIMER ***************************************************************************************
* Logs are prevented to avoid polluting the test output.
****************************************************************************************************
*/
// eslint-disable-next-line jest/no-hooks
beforeEach(() => {
jest.spyOn(console, 'warn').mockImplementation(() => { });
});

it('should remove all the internal keys', async() => {
const workload = new Workload(customWorkload, {
getters: { schemaFor: () => ({ linkFor: jest.fn() }) },
dispatch: jest.fn(),
rootGetters: { 'i18n/t': jest.fn() },
});
const expectation = { spec: { template: { spec: { containers: [{}], initContainers: [{}] } } } };

await workload.save();

expect({ ...workload }).toStrictEqual(expectation);
richard-cox marked this conversation as resolved.
Show resolved Hide resolved
});
});
});
});
30 changes: 30 additions & 0 deletions shell/models/pod.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { colorForState, stateDisplay } from '@shell/plugins/dashboard-store/reso
import { NODE, WORKLOAD_TYPES } from '@shell/config/types';
import { escapeHtml, shortenedImage } from '@shell/utils/string';
import WorkloadService from '@shell/models/workload.service';
import { NEVER_ADD_CONTAINER_FIELDS } from '@shell/utils/create-yaml';
import { unset } from 'lodash';

export const WORKLOAD_PRIORITY = {
[WORKLOAD_TYPES.DEPLOYMENT]: 1,
Expand Down Expand Up @@ -256,4 +258,32 @@ export default class Pod extends WorkloadService {
return Promise.reject(e);
});
}

removeContainerField(container) {
richard-cox marked this conversation as resolved.
Show resolved Hide resolved
for (const field of NEVER_ADD_CONTAINER_FIELDS) {
unset(container, field);
richard-cox marked this conversation as resolved.
Show resolved Hide resolved
}

return container;
}

cleanForSave(data) {
const val = super.cleanForSave(data);

// remove fields from array of spec.template.spec.containers
richard-cox marked this conversation as resolved.
Show resolved Hide resolved
if (val.spec?.containers) {
val.spec.containers.forEach((container) => {
this.removeContainerField(container);
});
}

// remove fields from initContainers
if (val.spec?.initContainers) {
val.spec.initContainers.forEach((container) => {
this.removeContainerField(container);
});
}

return val;
}
}
57 changes: 37 additions & 20 deletions shell/models/secret.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { SERVICE_ACCOUNT } from '@shell/config/types';
import { set } from '@shell/utils/object';
import { NAME as MANAGER } from '@shell/config/product/manager';
import SteveModel from '@shell/plugins/steve/steve-class';
import { NEVER_ADD } from '@shell/utils/create-yaml';
import { colorForState, stateDisplay, STATES_ENUM } from '@shell/plugins/dashboard-store/resource-class';
import { diffFrom } from '@shell/utils/time';
import day from 'dayjs';
Expand Down Expand Up @@ -49,7 +50,7 @@ export default class Secret extends SteveModel {
}

get issuer() {
const { metadata:{ annotations = {} } } = this;
const { metadata: { annotations = {} } } = this;
richard-cox marked this conversation as resolved.
Show resolved Hide resolved

if (annotations[CERTMANAGER.ISSUER]) {
return annotations[CERTMANAGER.ISSUER];
Expand Down Expand Up @@ -109,10 +110,10 @@ export default class Secret extends SteveModel {
}
];

if ( this._type === TYPES.SERVICE_ACCT ) {
if (this._type === TYPES.SERVICE_ACCT) {
const name = this.metadata?.annotations?.[KUBERNETES.SERVICE_ACCOUNT_NAME];

if ( name ) {
if (name) {
out.push({
label: 'Service Account',
formatter: 'LinkName',
Expand Down Expand Up @@ -153,11 +154,11 @@ export default class Secret extends SteveModel {
}

get canUpdate() {
if ( !this.hasLink('update') ) {
if (!this.hasLink('update')) {
return false;
}

if ( this._type === TYPES.SERVICE_ACCT ) {
if (this._type === TYPES.SERVICE_ACCT) {
return false;
}

Expand All @@ -170,7 +171,7 @@ export default class Secret extends SteveModel {
...Object.keys(this.binaryData || [])
];

if ( !keys.length ) {
if (!keys.length) {
return '(none)';
}

Expand Down Expand Up @@ -204,40 +205,40 @@ export default class Secret extends SteveModel {
}
} else if (this._type === TYPES.TLS) {
return this.certInfo || this.keysDisplay;
} else if ( this._type === TYPES.BASIC ) {
} else if (this._type === TYPES.BASIC) {
return base64Decode(this.data.username);
} else if ( this._type === TYPES.SSH ) {
} else if (this._type === TYPES.SSH) {
return this.sshUser;
} else if ( this._type === TYPES.SERVICE_ACCT ) {
} else if (this._type === TYPES.SERVICE_ACCT) {
return this.metadata?.annotations?.['kubernetes.io/service-account.name'];
}

return this.keysDisplay;
}

get sshUser() {
if ( this._type !== TYPES.SSH ) {
if (this._type !== TYPES.SSH) {
return null;
}

const pub = base64Decode(this.data['ssh-publickey']);

if ( !pub ) {
if (!pub) {
return null;
}

if ( pub.startsWith('----') ) {
if (pub.startsWith('----')) {
// PEM format
const match = pub.match(/from OpenSSH by ([^"]+)"/);

if ( match ) {
if (match) {
return match[1];
}
} else if ( pub.startsWith('ssh-') ) {
} else if (pub.startsWith('ssh-')) {
// OpenSSH format
const parts = pub.replace(/\n/g, '').split(/\s+/);

if ( parts && parts.length === 3 ) {
if (parts && parts.length === 3) {
return parts[2];
}
}
Expand Down Expand Up @@ -354,7 +355,7 @@ export default class Secret extends SteveModel {
get decodedData() {
const out = {};

for ( const k in this.data || {} ) {
for (const k in this.data || {}) {
out[k] = base64Decode(this.data[k]);
}

Expand All @@ -365,33 +366,49 @@ export default class Secret extends SteveModel {
return (key, value) => { // or (mapOfNewData)
const isMap = key && typeof key === 'object';

if ( !this.data || isMap ) {
if (!this.data || isMap) {
set(this, 'data', {});
}

let neu;

if ( isMap ) {
if (isMap) {
neu = key;
} else {
neu = { [key]: value };
}

for ( const k in neu ) {
for (const k in neu) {
// The key is quoted so that keys like '.dockerconfigjson' that contain dot don't get parsed into an object path
set(this.data, `"${ k }"`, base64Encode(neu[k]));
}
};
}

get doneRoute() {
if ( this.$rootGetters['currentProduct'].name === MANAGER ) {
if (this.$rootGetters['currentProduct'].name === MANAGER) {
return 'c-cluster-manager-secret';
} else {
return 'c-cluster-product-resource';
}
}

cleanForSave(data) {
// There are some properties in ACTIVELY_REMOVE
// that should be removed from the data
const val = super._cleanForSave(data);

const fieldsToRemove = [...NEVER_ADD];
richard-cox marked this conversation as resolved.
Show resolved Hide resolved

for (const field of fieldsToRemove) {
if (val[field] !== undefined) {
richard-cox marked this conversation as resolved.
Show resolved Hide resolved
delete val[field];
}
}

return val;
}

get certLifetime() {
if (this._type === TYPES.TLS) {
const certInfo = this.cachedCertInfo;
Expand Down
Loading
Loading