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(jqFilter)!: remove extra quotes around output parameter value #3251

Merged
merged 2 commits into from
Jun 23, 2020

Conversation

TTRh
Copy link
Contributor

@TTRh TTRh commented Jun 18, 2020

When playing with jqFilter, the return value begins with double quotes and ends in double quotes, also followed by a newline: i.e.:

 "parameter value"\n

Related PR : #1232

This commit ensure we do not insert extra double quotes and trailing new line when using
valueFrom: jqFilter to set the value of an output parameter for
resource templates.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed the CLA.
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My builds are green. Try syncing with master if they are not.
  • My organization is added to USERS.md.

Ensure we do not insert extra single quotes when using
valueFrom: jqFilter to set the value of an output parameter for
resource templates.
@CLAassistant
Copy link

CLAassistant commented Jun 18, 2020

CLA assistant check
All committers have signed the CLA.

@TTRh TTRh changed the title fix(resources): remove extra quotes around output parameter value fix(jqFilter): remove extra quotes around output parameter value Jun 18, 2020
@alexec
Copy link
Contributor

alexec commented Jun 18, 2020

test?

@TTRh
Copy link
Contributor Author

TTRh commented Jun 19, 2020

@alexec sure, i'm not super familiar with go but i can give it a try, could you point me to a test i could start from ?

@alexec
Copy link
Contributor

alexec commented Jun 19, 2020

workflow/executor/resource_test.go?

@sonarcloud
Copy link

sonarcloud bot commented Jun 23, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@TTRh
Copy link
Contributor Author

TTRh commented Jun 23, 2020

Hey :) at the end i create an e2e test workflow, making this workflow failing by comparing input parameters and expected parameters.
Tested with latest version of argo, it fails on the single value comparison:

 1,2c1                                                                                                                                                                                                                                                                          
< <"foo"
< >
---
> <foo>

@alexec alexec changed the title fix(jqFilter): remove extra quotes around output parameter value fix(jqFilter)!: remove extra quotes around output parameter value Jun 23, 2020
@alexec
Copy link
Contributor

alexec commented Jun 23, 2020

I've marked an "!" as maybe breaking in some cases.

@alexec alexec merged commit e6aab60 into argoproj:master Jun 23, 2020
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.

3 participants