dotnet / docker-tools

This is a repo to house some common tools for our various docker repos.
MIT License
122 stars 46 forks source link

Fix staging server variable reference for PowerShell #1336

Closed mthalman closed 3 months ago

mthalman commented 3 months ago

The use of $env:ACR-STAGING_SERVER is incorrect syntax. It results in PowerShell looking for an environment variable named ACR and then just appends the string -STAGING_SERVER to the result because - is a variable delimiter. Since there is no ACR variable, the result of this is -STAGING_SERVER which obviously isn't what we want.

Fixed to use {..} syntax to include the entire variable name.

dotnet-issue-labeler[bot] commented 3 months ago

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

dotnet-issue-labeler[bot] commented 3 months ago

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

ellahathaway commented 3 months ago

Thanks for fixing! Can you explain how you discovered this? I went back over the test build I did with the initial (breaking) change, and I'm struggling to determine where the error occurred. I'd like to know what testing I could've done to catch this error earlier.

mthalman commented 3 months ago

Thanks for fixing! Can you explain how you discovered this? I went back over the test build I did with the initial (breaking) change, and I'm struggling to determine where the error occurred. I'd like to know what testing I could've done to catch this error earlier.

I discovered it in this build (internal link). That build attempts to use the value of the staging server which then gets the error. In your test build, it doesn't pull any images so it doesn't run into the problem. The Image Builder pipeline is unique in that way. It just runs unit tests against the project and doesn't actually test any built images. That's my bad for not realizing this and letting you know.