-
Notifications
You must be signed in to change notification settings - Fork 100
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
Update credential precedence in bicepconfig.json #7803
Conversation
Signed-off-by: sk593 <[email protected]>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
667c23d
to
1d7b4d0
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
1d7b4d0
to
3a8df14
Compare
Signed-off-by: sk593 <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7803 +/- ##
==========================================
- Coverage 61.07% 61.05% -0.03%
==========================================
Files 523 523
Lines 27446 27446
==========================================
- Hits 16762 16756 -6
- Misses 9204 9207 +3
- Partials 1480 1483 +3 ☔ View full report in Codecov by Sentry. |
aae53b1
to
e20fcec
Compare
f7bef28
to
3bba88b
Compare
3bba88b
to
f62e9cb
Compare
Signed-off-by: sk593 <[email protected]>
f091c18
to
f62e9cb
Compare
Signed-off-by: sk593 <[email protected]>
@@ -314,6 +314,64 @@ jobs: | |||
append: true | |||
message: | | |||
:hourglass: Publishing Bicep Recipes for functional tests... | |||
|
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.
Moving this into the build job so we make sure to upload any bicep changes before pushing recipes
bfabe47
to
850243a
Compare
Signed-off-by: sk593 <[email protected]>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Signed-off-by: sk593 <[email protected]>
Signed-off-by: sk593 <[email protected]>
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
- name: Upload Radius Bicep types artifacts | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: ${{ matrix.name }}_radius_bicep_types |
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.
matrix is not reachable here
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.
Updated
Signed-off-by: sk593 <[email protected]>
run: | | ||
bicep publish-extension ./hack/bicep-types-radius/generated/index.json --target br:${{ env.BICEP_TYPES_REGISTRY }}/test/radius:${{ env.REL_VERSION == 'edge' && 'latest' || env.REL_VERSION }} --force | ||
|
||
- name: Generate test bicepconfig.json |
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.
We have of two of these in this workflow. Is this expected?
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.
yes, the files dont persist between jobs so we need it in both places. the first one is part of the workaround because we need to disable azure auth before we publish the bicep test recipes (this will eventually be removed once the auth bug is fixed) and the second is so we can test against the PR versions of the types (in the event that there are bicep changes in a PR)
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.
Do we need the step that prints out the details of the JSON file in production?
- name: Print updated bicepconfig.json | ||
run: cat bicepconfig.json |
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.
Do we need this in production?
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.
No, this is in place of the initial workaround since there were authentication issues with the ACR even though we have anonymous pulls. The issue is that when we publish to ACR, we have to authenticate so only having "Environment" authentication removed any Azure CLI auth. Instead, we'll add the "Environment" workaround when we want to disable azure auth. Ideally, we can remove this everywhere in the repo once the bug is fixed: #7804
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.
lgtm
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
# Description Some workflows need Az authentication and some don't. This removes the default az auth in workflows anytime its not needed ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request fixes a bug in Radius and has an approved issue (issue link required). - This pull request adds or changes features of Radius and has an approved issue (issue link required). - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). <!-- Please update the following to link the associated issue. This is required for some kinds of changes (see above). --> Fixes: #issue_number --------- Signed-off-by: sk593 <[email protected]>
# Description Some workflows need Az authentication and some don't. This removes the default az auth in workflows anytime its not needed ## Type of change <!-- Please select **one** of the following options that describes your change and delete the others. Clearly identifying the type of change you are making will help us review your PR faster, and is used in authoring release notes. If you are making a bug fix or functionality change to Radius and do not have an associated issue link please create one now. --> - This pull request fixes a bug in Radius and has an approved issue (issue link required). - This pull request adds or changes features of Radius and has an approved issue (issue link required). - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). <!-- Please update the following to link the associated issue. This is required for some kinds of changes (see above). --> Fixes: #issue_number --------- Signed-off-by: sk593 <[email protected]> Signed-off-by: Reshma Abdul Rahim <[email protected]>
Description
Some workflows need Az authentication and some don't. This removes the default az auth in workflows anytime its not needed
Type of change
Fixes: #issue_number