Skip to content

Commit

Permalink
Changes after review
Browse files Browse the repository at this point in the history
- force fetch of secrets on visit on install app/chart detail/app detail page
- add somre jsdoc


Note
This shouldn't break extensions UNLESS extension developers have implemented
custom chart install/upgrade processes. Existing extensions like kubewarden
or elemental are fine (they redirect to actual chart install process rather
than have their own)
  • Loading branch information
richard-cox committed Oct 28, 2024
1 parent 90b40d9 commit 0ad5b03
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 45 deletions.
2 changes: 1 addition & 1 deletion shell/detail/catalog.cattle.io.app.vue
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export default {
const promises = {
catalog: this.$store.dispatch('catalog/load'),
allOperations: this.$store.dispatch('cluster/findAll', { type: CATALOG.OPERATION }),
secrets: this.value?.fetchValues(),
secrets: this.value.fetchValues(true),
};
const res = await allHash(promises);
Expand Down
4 changes: 2 additions & 2 deletions shell/mixins/chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export default {
id: `${ this.query.appNamespace }/${ this.query.appName }`,
});

await this.existing?.fetchValues();
await this.existing?.fetchValues(true);

this.mode = _EDIT;
} catch (e) {
Expand Down Expand Up @@ -452,7 +452,7 @@ export default {
}
}
if (existingCRDApp) {
await existingCRDApp.fetchValues();
await existingCRDApp.fetchValues(true);

// spec.values are any non-default values the user configured
// the installation form should show these, as well as any default values from the chart
Expand Down
80 changes: 42 additions & 38 deletions shell/models/catalog.cattle.io.app.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,53 +305,51 @@ export default class CatalogApp extends SteveModel {

_values = undefined;

/*
* Value live in a helm secret, so fetch it (with special param) and cache locally
*/
async fetchValues() {
if (this._values) {
/**
* User and Chart values live in a helm secret, so fetch it (with special param) and cache them locally
*/
async fetchValues(force = false) {
if (this._values && !force) {
return;
}

if (this._values === undefined) {
const metadata = this.metadata;
const secretReference = metadata.ownerReferences?.find((ow) => ow.kind.toLowerCase() === SECRET);

const secretId = secretReference?.name;
const secretNamespace = metadata.namespace;
if (this._values === null) {
// Note - if this._values is null this isn't ever going to work, no need to carry on
return;
}

if (!secretNamespace || !secretId) {
console.warn(`Cannot find values for ${ this.id } (cannot find related secret namespace or id)`); // eslint-disable-line no-console
const metadata = this.metadata;
const secretReference = metadata.ownerReferences?.find((ow) => ow.kind.toLowerCase() === SECRET);

return null;
}
const secretId = secretReference?.name;
const secretNamespace = metadata.namespace;

try {
const id = `${ secretNamespace }/${ secretId }`;
const existingSecret = this.$getters['byId'](SECRET, id);
const haveValues = !!existingSecret?.data?.release?.chart?.values && !!existingSecret?.data?.release?.config;
const secret = haveValues ? existingSecret : await this.$dispatch('find', {
type: SECRET,
id,
opt: {
force: !!existingSecret, // force if we have a secret without the values in (Secret has been fetched another way)
watch: false, // Cannot watch with custom params (they are dropped on calls made when resyncing over socket)
params: { includeHelmData: true }
}
});
if (!secretNamespace || !secretId) {
console.warn(`Cannot find values for ${ this.id } (cannot find related secret namespace or id)`); // eslint-disable-line no-console

this._values = {
values: { ...secret.data?.release?.config },
chartValues: { ...secret.data?.release?.chart?.values }
};
return null;
}

// Avoid undefined
return;
} catch (e) {
console.error(`Cannot find values for ${ this.id } (unable to fetch)`, e); // eslint-disable-line no-console
}
try {
const id = `${ secretNamespace }/${ secretId }`;
const existingSecret = this.$getters['byId'](SECRET, id);
const haveValues = !!existingSecret?.data?.release?.chart?.values && !!existingSecret?.data?.release?.config;
const secret = haveValues ? existingSecret : await this.$dispatch('find', {
type: SECRET,
id,
opt: {
force: force || !!existingSecret, // force if we have a secret without the values in (Secret has been fetched another way)
watch: false, // Cannot watch with custom params (they are dropped on calls made when resyncing over socket)
params: { includeHelmData: true }
}
});

return undefined;
this._values = {
values: { ...secret.data?.release?.config },
chartValues: { ...secret.data?.release?.chart?.values }
};
} catch (e) {
console.error(`Cannot find values for ${ this.id } (unable to fetch)`, e); // eslint-disable-line no-console
}
}

Expand All @@ -372,12 +370,18 @@ export default class CatalogApp extends SteveModel {
}
}

/**
* The user's helm values
*/
get values() {
this._validateValuesSecret('values');

return this._values.values;
}

/**
* The Charts default helm values
*/
get chartValues() {
this._validateValuesSecret('chartValues');

Expand Down
6 changes: 2 additions & 4 deletions shell/pages/c/_cluster/apps/charts/install.vue
Original file line number Diff line number Diff line change
Expand Up @@ -1010,10 +1010,8 @@ export default {
const res = await this.repo.doAction((isUpgrade ? 'upgrade' : 'install'), input);
if (isUpgrade && this.existing) {
// The chart values are cached locally. Clear them so we refetch from secret again
this.existing.clearValues();
}
// The chart values are cached locally. Clear them so we refetch from secret again
this.existing?.clearValues();
const operationId = `${ res.operationNamespace }/${ res.operationName }`;
Expand Down

0 comments on commit 0ad5b03

Please sign in to comment.