Skip to content

Commit

Permalink
Fix issue where socket updates to helm app wiped cached values
Browse files Browse the repository at this point in the history
- socket updates remove all properties from an object on update, this included `_values`
- switch to more solit / response secrets getter instead

Top notes
- Helm app references secret by owner refs
- helm repo update action supplies helm app update and receives references to helm operationin response
  - This creates a new secret and updates the helm app's secret owner reference
- ui receives socket up to helm app containing new secret owner ref
  • Loading branch information
richard-cox committed Oct 28, 2024
1 parent 0ad5b03 commit 8a7633f
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 44 deletions.
6 changes: 6 additions & 0 deletions shell/detail/catalog.cattle.io.app.vue
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ export default {
}
},
},
watch: {
'value.secretId'(neu, old) {
this.value.fetchValues(true);
}
},
};
</script>
Expand Down
82 changes: 42 additions & 40 deletions shell/models/catalog.cattle.io.app.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,21 +303,38 @@ export default class CatalogApp extends SteveModel {
return false;
}

_values = undefined;

/**
* User and Chart values live in a helm secret, so fetch it (with special param) and cache them locally
* User and Chart values live in a helm secret, so fetch it (with special param)
*/
async fetchValues(force = false) {
if (this._values && !force) {
if (!this.secretId) {
// If there's no secret id this isn't ever going to work, no need to carry on
return;
}

if (this._values === null) {
// Note - if this._values is null this isn't ever going to work, no need to carry on
const haveValues = !!this._values && !!this._chartValues;

if (haveValues && !force) {
// If we already have the required values and we're not forced to re-fetch, no need to carry on
return;
}

try {
await this.$dispatch('find', {
type: SECRET,
id: this.secretId,
opt: {
force: force || (!!this._secret && !haveValues), // force if explicitly requested or there's ean existing secret without the required values 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 }
}
});
} catch (e) {
console.error(`Cannot find values for ${ this.id } (unable to fetch)`, e); // eslint-disable-line no-console
}
}

get secretId() {
const metadata = this.metadata;
const secretReference = metadata.ownerReferences?.find((ow) => ow.kind.toLowerCase() === SECRET);

Expand All @@ -330,62 +347,47 @@ export default class CatalogApp extends SteveModel {
return null;
}

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 }
}
});

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
}
return `${ secretNamespace }/${ secretId }`;
}

/**
* Clear cached helm secret based values. This will force a refetch when fetchValues is called again
*/
clearValues() {
delete this._values;
get _secret() {
return this.secretId ? this.$getters['byId'](SECRET, this.secretId) : null;
}

_validateValuesSecret(noun) {
if (this._values === undefined) {
_validateSecret(noun) {
if (this._secret === undefined) {
throw new Error(`Cannot find ${ noun } for ${ this.id } (chart secret has not been fetched via app \`fetchValues\`)`);
}

if (this._values === null) {
throw new Error(`Cannot find ${ noun } for ${ this.id } (chart secret failed to fetch) `);
if (this._secret === null) {
throw new Error(`Cannot find ${ noun } for ${ this.id } (chart secret cannot or has failed to fetch) `);
}
}

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

return this._values.values;
return this._values;
}

get _values() {
return this._secret?.data?.release?.config;
}

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

return this._chartValues;
}

return this._values.chartValues;
get _chartValues() {
return this._secret?.data?.release?.chart?.values;
}
}

Expand Down
4 changes: 0 additions & 4 deletions shell/pages/c/_cluster/apps/charts/install.vue
Original file line number Diff line number Diff line change
Expand Up @@ -1009,10 +1009,6 @@ export default {
}
const res = await this.repo.doAction((isUpgrade ? 'upgrade' : 'install'), input);
// The chart values are cached locally. Clear them so we refetch from secret again
this.existing?.clearValues();
const operationId = `${ res.operationNamespace }/${ res.operationName }`;
// Non-admins without a cluster won't be able to fetch operations immediately
Expand Down

0 comments on commit 8a7633f

Please sign in to comment.