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

fix: Job String Processor, Job Processor and support for encoding of variables. #235

Conversation

merlin-northern
Copy link
Contributor

No description provided.

@mender-test-bot
Copy link

@merlin-northern, Let me know if you want to start the integration pipeline by mentioning me and the command "start pipeline".


my commands and options
You can trigger a pipeline on multiple prs with:
  • mentioning me and start pipeline --pr mender/127 --pr mender-connect/255

You can start a fast pipeline, disabling full integration tests with:

  • mentioning me and start pipeline --fast

You can cherry pick to a given branch or branches with:

  • mentioning me and:
 cherry-pick to:
 * 1.0.x
 * 2.0.x

@merlin-northern
Copy link
Contributor Author

@mender-test-bot start pipeline sugar please

@mender-test-bot
Copy link

Hello 😸 I created a pipeline for you here: Pipeline-633779599

Build Configuration Matrix

Key Value
AUDITLOGS_REV master
BUILD_CLIENT false
CREATE_ARTIFACT_WORKER_REV master
DEPLOYMENTS_ENTERPRISE_REV master
DEPLOYMENTS_REV master
DEVICEAUTH_ENTERPRISE_REV master
DEVICEAUTH_REV master
DEVICECONFIG_REV master
DEVICECONNECT_REV master
DEVICEMONITOR_REV master
GUI_REV master
INTEGRATION_REV master
INVENTORY_ENTERPRISE_REV master
INVENTORY_REV master
IOT_MANAGER_REV master
MENDER_ARTIFACT_REV master
MENDER_BINARY_DELTA_REV master
MENDER_CLI_REV master
MENDER_CONFIGURE_MODULE_REV master
MENDER_CONNECT_REV master
MENDER_CONVERT_REV master
MENDER_GATEWAY_REV master
MENDER_REV master
MONITOR_CLIENT_REV master
MTLS_AMBASSADOR_REV master
REPORTING_REV master
RUN_BACKEND_INTEGRATION_TESTS true
RUN_INTEGRATION_TESTS true
TENANTADM_REV master
USERADM_ENTERPRISE_REV master
USERADM_REV master
WORKFLOWS_ENTERPRISE_REV master
WORKFLOWS_REV pull/235/head

@merlin-northern merlin-northern changed the title Men 5819 escape query params processor alf Men 5819 escape query params processor Sep 7, 2022
Copy link
Contributor

@alfrunes alfrunes left a comment

Choose a reason for hiding this comment

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

LGTM. Should we also update the affected workflows adding the correct encoding to query parameters?

@merlin-northern
Copy link
Contributor Author

merlin-northern commented Sep 8, 2022

LGTM. Should we also update the affected workflows adding the correct encoding to query parameters?

part of the beauty of one of the previous versions was: by default we encode in the URIs :> so we would not have to do it. I was about to update only in one place in enterprise, since in here there are only ids, scopes and statuses -- all of which will never work with non-URI permitted characters. But I will do it, for clarity, and for documentation :)

Edit: 8444ca8

@merlin-northern
Copy link
Contributor Author

@mender-test-bot start pipeline sugar pretty please honey

@mender-test-bot
Copy link

Hello 😸 I created a pipeline for you here: Pipeline-634344874

Build Configuration Matrix

Key Value
AUDITLOGS_REV master
BUILD_CLIENT false
CREATE_ARTIFACT_WORKER_REV master
DEPLOYMENTS_ENTERPRISE_REV master
DEPLOYMENTS_REV master
DEVICEAUTH_ENTERPRISE_REV master
DEVICEAUTH_REV master
DEVICECONFIG_REV master
DEVICECONNECT_REV master
DEVICEMONITOR_REV master
GUI_REV master
INTEGRATION_REV master
INVENTORY_ENTERPRISE_REV master
INVENTORY_REV master
IOT_MANAGER_REV master
MENDER_ARTIFACT_REV master
MENDER_BINARY_DELTA_REV master
MENDER_CLI_REV master
MENDER_CONFIGURE_MODULE_REV master
MENDER_CONNECT_REV master
MENDER_CONVERT_REV master
MENDER_GATEWAY_REV master
MENDER_REV master
MONITOR_CLIENT_REV master
MTLS_AMBASSADOR_REV master
REPORTING_REV master
RUN_BACKEND_INTEGRATION_TESTS true
RUN_INTEGRATION_TESTS true
TENANTADM_REV master
USERADM_ENTERPRISE_REV master
USERADM_REV master
WORKFLOWS_ENTERPRISE_REV master
WORKFLOWS_REV pull/235/head

@merlin-northern merlin-northern force-pushed the men_5819_escape_query_params_processor_alf branch 2 times, most recently from 8537ce6 to 8444ca8 Compare September 8, 2022 08:48
@alfrunes alfrunes self-requested a review September 8, 2022 09:13
@alfrunes
Copy link
Contributor

alfrunes commented Sep 8, 2022

Can you please fix the commits?

  • The first two commits can either be squashed into one commit or separated into one refac and one feat commit separating the options/flag feature from the refactoring.
    • If you decide to squash, please add a feat tag with a more descriptive changelog describing the feature (adding options/flags to variable expansions).
  • The last commit should be a fix and contain a changelog.

@merlin-northern merlin-northern force-pushed the men_5819_escape_query_params_processor_alf branch from 8444ca8 to 41f8b79 Compare September 8, 2022 13:20
…variables.

* refactors code around processors
* allows to specify ${encoding=url;workflow.input.identifier}
* sets explicit encoding to url for all input values passed in the URI
* the extension of regular expressions by Alf see workflows/pull/235,
  workflows/pull/234 with further fixes

Changelog: Support for flags insize the ${workflow.input.identifier} via ${encoding=url;workflow.input.identifier}
Changelog: All workflows use encoding=url in the input variables passed in the URIs.
Ticket: MEN-5819
Signed-off-by: Peter Grzybowski <peter@northern.tech>
@merlin-northern merlin-northern force-pushed the men_5819_escape_query_params_processor_alf branch from 41f8b79 to 3787ecb Compare September 8, 2022 13:26
@merlin-northern
Copy link
Contributor Author

not running the pipeline again, there were only squashes and commit msgs fixes.

@merlin-northern merlin-northern changed the title Men 5819 escape query params processor fix: Job String Processor, Job Processor and support for encoding of variables. Sep 8, 2022
@merlin-northern merlin-northern merged commit e31e7fb into mendersoftware:master Sep 8, 2022
@mender-test-bot
Copy link

Hello 😸 This PR contains changelog entries. Please, verify the need of backporting it to the following release branches:
2.2.x (release 3.3.x) - 🤖 🍒
2.0.x (release 3.0.x)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants