-
Notifications
You must be signed in to change notification settings - Fork 23
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 pipeline create templates #261
Conversation
🦙 MegaLinter status: ❌ ERROR
See detailed report in MegaLinter reports |
Modules tests Results3 tests 3 ✅ 8m 14s ⏱️ Results for commit 95071d3. |
Creating environment with name: "10bdf0" using commit: "95071d32c63c74639ff630e15949e2ee460b0c87". The environment will be destroyed after the workflow completes. |
E2E Tests Results1 tests 1 ✅ 1m 32s ⏱️ Results for commit 95071d3. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the fix. lgtm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change provides invalid json
syntax. Expected is to use quote.
Any linter will block this, like in this case with checks: https://github.com/microsoft/symphony/actions/runs/8622793305/job/23634701234#step:5:189
Instead of producing invalid/non-standard json, let's fix the issue in script where root-cause is, like this example: https://github.com/microsoft/symphony/blob/main/scripts/install/providers/azdo/azdo.sh#L368
is
sed <"$SCRIPT_DIR/templates/pipeline-authorize.json" 's~__PIPELINE_ID__~'"${_pipeId}"'~'
should be
sed <"$SCRIPT_DIR/templates/pipeline-authorize.json" 's~__PIPELINE_ID__~'${_pipeId}'~'
@DariuszPorowski - We actually started going down this path, but the sed command is slightly more complicated. Adding a quote like so breaks the interpolation. In this case it really comes down that the fact that json file isn't a json file, it's a template file. So it shouldn't be considered json and not subject to linting rules. Put another way, it's expected that it's not valid json. We will add a future story to move these to from json to .json.tmpl template files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to fix in the future with templates: #261 (comment)
This PR fixes a bug when running Symphony CLI cmd: symphony pipeline config for Azure devops where pipelines were creating without the expected variables.
I tested the changes and where able to successfully configure an azure devops repo for terraform and run the Deploy pipeline.