From c60b3b7af82103cc32606e021680a0213aa3efcd Mon Sep 17 00:00:00 2001 From: blam Date: Tue, 26 Mar 2024 15:28:07 +0100 Subject: [PATCH 01/10] chore: creating the initial bep Signed-off-by: blam --- .../README.md | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 beps/NNNN-scaffolder-templating-in-parameters/README.md diff --git a/beps/NNNN-scaffolder-templating-in-parameters/README.md b/beps/NNNN-scaffolder-templating-in-parameters/README.md new file mode 100644 index 0000000000..49ec16d8ed --- /dev/null +++ b/beps/NNNN-scaffolder-templating-in-parameters/README.md @@ -0,0 +1,100 @@ +--- +title: Supporting templating syntax in `parameters` +status: provisional +authors: + - '@benjdlambert' +owners: + - '@benjdlambert' + - '@backstage/scaffolder-maintainers' +project-areas: + - scaffolder +creation-date: 2024-03-26 +--- + + + +# BEP: + + + +[**Discussion Issue**](https://github.com/backstage/backstage/issues/NNNNN) + +- [Summary](#summary) +- [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) +- [Design Details](#design-details) +- [Release Plan](#release-plan) +- [Dependencies](#dependencies) +- [Alternatives](#alternatives) + +## Summary + + + +## Motivation + + + +### Goals + + + +### Non-Goals + + + +## Proposal + + + +## Design Details + + + +## Release Plan + + + +## Dependencies + + + +## Alternatives + + From 8f6356169c85bb6f14b8ae0d5fdeb1838df5dbe1 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 28 Mar 2024 17:45:53 +0100 Subject: [PATCH 02/10] chore: starting to write some more for the bep Signed-off-by: blam Signed-off-by: blam --- .../README.md | 46 +++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/beps/NNNN-scaffolder-templating-in-parameters/README.md b/beps/NNNN-scaffolder-templating-in-parameters/README.md index 49ec16d8ed..71f92e9b64 100644 --- a/beps/NNNN-scaffolder-templating-in-parameters/README.md +++ b/beps/NNNN-scaffolder-templating-in-parameters/README.md @@ -1,5 +1,5 @@ --- -title: Supporting templating syntax in `parameters` +title: Supporting templating syntax in `parameters` schema status: provisional authors: - '@benjdlambert' @@ -17,11 +17,11 @@ creation-date: 2024-03-26 When editing BEPs, aim for tightly-scoped, single-topic PRs to keep discussions focused. If you disagree with what is already in a document, open a new PR with suggested changes. --> -# BEP: +# BEP: Supporting templating syntax in `parameters` schema -[**Discussion Issue**](https://github.com/backstage/backstage/issues/NNNNN) +[**Discussion Issue**](https://github.com/backstage/backstage/issues/16275) - [Summary](#summary) - [Motivation](#motivation) @@ -39,6 +39,30 @@ When editing BEPs, aim for tightly-scoped, single-topic PRs to keep discussions The summary of the BEP is a few paragraphs long and give a high-level overview of the features to be implemented. It should be possible to read *only* the summary and understand what the BEP is proposing to accomplish and what impact it has for users. --> +This BEP proposes to add support for templating syntax in the `parameters` schema of a scaffolder template. +This will allow users to define properties in the JSON Schema which are templated from current values that have been collected from the user already. +This can be useful when you want to use a value that has already been collected as a default value in another field. + +For example: + +```yaml +apiVersion: scaffolder.backstage.io/v1beta3 +kind: Template +metadata: + name: my-template +spec: + parameters: + - title: Some input + description: Get some info from the user + properties: + name: + type: string + default: Test + description: + type: string + default: ${{ parameters.name or "unknown" }}-description +``` + ## Motivation +Inclusive of the initial RFC there's been a swarm of issues that are requesting this feature, and we want to align on the implementation and design of this feature. + +See the following: + +- https://github.com/backstage/backstage/issues/16275 +- https://github.com/backstage/backstage/pull/23283 +- https://github.com/backstage/backstage/issues/19597 +- https://github.com/backstage/backstage/issues/20533 +- https://github.com/backstage/backstage/pull/17746 + +There's some ideas for introducing a templating syntax for both templating into the `parameters` schema, and also being able to pass through some templating strings to underlying field extensions that can use those templating strings. +We want to align here so that we're not going to have those conflict or compete, and create a standard for how to achieve templating in both circumstances. + ### Goals +- This BEP will settle the implementation for the templating of fields into the JSON Schema in the `parameters` section in the scaffolder templates. +- This BEP will settle how to pass through templating strings to underlying field extensions in a non-conflicting way. + ### Non-Goals +The proposal is to be able to decorate the template schema server side with a context and use that to drive the form rendering client side. + +We can extend the `/parameter-schema` endpoint to accept a `formData` context query parameter which will be a JSON object of the current `formData` state. + +```diff +export interface ScaffolderApi { + getTemplateParameterSchema( + templateRef: string, ++ formData?: string, + ): Promise; +} +``` + +```diff + router + .get( + '/v2/templates/:namespace/:kind/:name/parameter-schema', + async (req, res) => { + const credentials = await httpAuth.credentials(req); + const { token } = await auth.getPluginRequestToken({ + onBehalfOf: credentials, + targetPluginId: 'catalog', + }); + const template = await authorizeTemplate( + req.params, + token, + credentials, + ); + + const parameters = [template.spec.parameters ?? []].flat(); ++ const secureTemplater = await SecureTemplater.loadRenderer({ ++ templateFilters: { ++ ...createDefaultFilters({ integrations }), ++ ...additionalTemplateFilters, ++ }, ++ templateGlobals: additionalTemplateGlobals, ++ }); ++ ++ const templatedParameters = parameters.map(parameter => ++ renderTemplateString( ++ parameter, ++ { ++ parameters: req.query.formData, ++ }, ++ secureTemplater, ++ logger, ++ ), ++ ); +``` + +You can see a quick implementation of this in this [branch](https://github.com/backstage/backstage/compare/master...blam/templating-in-parameters) ## Design Details - +There's a slight issue with the implementation of the `react-jsonschema-form`, which makes things like live updating on things like the `default` field slightly more difficult. +Currently, on first render, the default value is populated and then stored in the `formData` object or the current state, and the default value is never re-evaluated again at a later stage. + +This means that if end users are wanting to set default values with `${{ parameters.myOtherProperty }}`, then they would need to ensure that they are on different steps in the form +as the form would need to be re-rendered, and for performance reasons, we don't want to re-render the form on every `formData` update. + +We could fix this, by implementing custom logic for when the `parameter-schema` is updated, if the updated field is in a `default: *` field, then we replace the previous value with the new value in the `formData` automatically. +This is a pretty ugly workaround, but maybe the only option we have. Also at this point, pretty unsure if this affects any other parts of the `JSONSchema`, and we would also have to implement it for those fields if they exist. + +Perhaps we just accept this as a limitation, and document it as such. ## Release Plan @@ -125,6 +175,8 @@ This section should describe the rollout process for any new features. It must t If there is any particular feedback to be gathered during the rollout, this should be described here as well. --> +This change is backwards compatible, and can be released in a minor release. There's no breaking changes to worry about here. + ## Dependencies + +#### Templating client side + +- This could lead to confusion as `filters` such as `parseRepoUrl` and `pick` and any custom filters which you define in the backend would not be available in the client side. +- Also with the limitations of the `default` value being updated only on first render and never re-evaluated, there's no performance benefit of doing things client side anymore. From 8db1ba1508ee0db37cb8c2690e59df43a018adee Mon Sep 17 00:00:00 2001 From: blam Date: Mon, 27 May 2024 09:49:25 +0200 Subject: [PATCH 06/10] chore: give the bep a number and feelings Signed-off-by: blam --- .../README.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename beps/{NNNN-scaffolder-templating-in-parameters => 0009-scaffolder-templating-in-parameters}/README.md (100%) diff --git a/beps/NNNN-scaffolder-templating-in-parameters/README.md b/beps/0009-scaffolder-templating-in-parameters/README.md similarity index 100% rename from beps/NNNN-scaffolder-templating-in-parameters/README.md rename to beps/0009-scaffolder-templating-in-parameters/README.md From 1c94fb14db0fd4df52356636f54dcafd5fd64c92 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 25 Jul 2024 08:52:31 +0200 Subject: [PATCH 07/10] chore: fix Signed-off-by: blam --- .../README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/beps/0009-scaffolder-templating-in-parameters/README.md b/beps/0009-scaffolder-templating-in-parameters/README.md index 450538ab8a..d5fc78497c 100644 --- a/beps/0009-scaffolder-templating-in-parameters/README.md +++ b/beps/0009-scaffolder-templating-in-parameters/README.md @@ -167,6 +167,20 @@ This is a pretty ugly workaround, but maybe the only option we have. Also at thi Perhaps we just accept this as a limitation, and document it as such. +Templating for `errorMessages` has been solved by using the `ajv-errors` library https://github.com/backstage/backstage/pull/25624, you can see more about [backrefs and pointers here](https://ajv.js.org/packages/ajv-errors.html). Any other template strings that will be passed through the underlying components and to be left untemplated should be encapsulated with options instead of passing through raw strings. The below example illustrates an `entityAndName` format, which under the hood, might do something like `${{ parameters.entity }} - ${{ parameters.name }}`, but this implementation never leaks out to the templating language. + +```yaml +parameters: + properties: + ... + description: + type: string + default: Test-description + ui:field: CustomDisplayField + ui:options: + format: entityAndName +``` + ## Release Plan