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

[Container Instances] The example of YAML for deployment is incorrect. #122214

Closed
mag-chang opened this issue May 3, 2024 · 5 comments
Closed

Comments

@mag-chang
Copy link

Hello team,

I want to let you know the document has incorrect section.
This document is showing an example of YAML for GitHub Actions to deploy to ACR.
However, the yaml is incorrect. I am going to explain about it at following.

  1. The step of 'Build and push image' has incorrect outer brackets for registry. $({ secrets.REGISTRY_LOGIN_SERVER })
  2. The username and password have missing secrets. These should be pointed as REGISTRY_USERNAME and REGISTRY_PASSWORD.

I would happy to check it once.

Thanks,


Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

@PesalaPavan
Copy link
Contributor

@mag-chang
Thanks for your feedback! We will investigate and update as appropriate.

@tomvcassidy
Copy link
Contributor

tomvcassidy commented May 6, 2024

Hi @mag-chang! Thank you for catching these issues. Below are the changes I plan to make:

Current:
- name: 'Build and push image'
uses: docker/login-action@v3
with:
registry: $({ secrets.REGISTRY_LOGIN_SERVER })
username: ${{ secrets.AZURE_CLIENT_ID }}
password: ${{ secrets.AZURE_CLIENT_SECRET }}

Proposed:
- name: 'Build and push image'
uses: docker/login-action@v3
with:
registry: ${{ secrets.REGISTRY_LOGIN_SERVER }}
username: ${{ secrets.REGISTRY_USERNAME }}
password: ${{ secrets.REGISTRY_PASSWORD }}

Please let me know if these corrections address the issues you've identified or if I've missed anything. Thank you!

@mag-chang
Copy link
Author

Hello @tomvcassidy ,
Thank you for your fixing the issue! I think the changes are appropriate.
I appreciate your quick action.

@PesalaPavan
Copy link
Contributor

@mag-chang
We are going to close this thread as resolved but if there are any further questions regarding the documentation, please tag me in your reply and we will be happy to continue the conversation.

@tomvcassidy
Copy link
Contributor

Thank you, all. The changes are merged as of https://github.com/MicrosoftDocs/azure-docs-pr/pull/274510 and will be published later today.

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

No branches or pull requests

3 participants