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

Bad error message when showing recipes in ACR #6902

Closed
Gijsreyn opened this issue Dec 2, 2023 · 16 comments · Fixed by #6974
Closed

Bad error message when showing recipes in ACR #6902

Gijsreyn opened this issue Dec 2, 2023 · 16 comments · Fixed by #6974
Labels
bug Something is broken or not working as expected good first issue Good for newcomers triaged This issue has been reviewed and triaged

Comments

@Gijsreyn
Copy link

Gijsreyn commented Dec 2, 2023

Bug information

Steps to reproduce (required)

  • Login with ACR login
  • Publish by running rad bicep publish
  • Run the following command
rad recipe show <recipeName> --resource-type <resourceType>

Observed behavior (required)

\"<acrName>.azurecr.io/azure/sql:latest\": HEAD \"https://<acrName>.azurecr.io/v2/azure/<recipeName>/manifests/latest\": GET \"https://<acrName>.azurecr.io/oauth2/token?scope=repository%3Aazure%2F<recipeName>%3Apull\u0026service=<acrName>.azurecr.io\": response status code 401: unauthorized: authentication required, visit https://aka.ms/acr/authorization for more information."

Desired behavior (required)

A better error message that explains the problem and tells you how to login to pull. If I were an external customer I wouldn't be sure how to fix this.

AB#10641

@Gijsreyn Gijsreyn added the bug Something is broken or not working as expected label Dec 2, 2023
@radius-triage-bot
Copy link

👋 @Gijsreyn Thanks for filing this bug report.

A project maintainer will review this report and get back to you soon. If you'd like immediate help troubleshooting, please visit our Discord server.

For more information on our triage process please visit our triage overview

@vinayada1
Copy link
Contributor

Thanks @Gijsreyn for submitting this issue. We will look into this shortly.

@Gijsreyn
Copy link
Author

Gijsreyn commented Dec 5, 2023

Thanks @Gijsreyn for submitting this issue. We will look into this shortly.

Would love to see it. Was trying to setup a demo from an operator perspective, but couldn't go further with the diagnostics Radius is proving.

@shalabhms shalabhms added the triaged This issue has been reviewed and triaged label Dec 7, 2023
@radius-triage-bot
Copy link

👍 We've reviewed this issue and have agreed to add it to our backlog. Please subscribe to this issue for notifications, we'll provide updates when we pick it up.

We also welcome community contributions! If you would like to pick this item up sooner and submit a pull request, please visit our contribution guidelines and assign this to yourself by commenting "/assign" on this issue.

For more information on our triage process please visit our triage overview

@shalabhms shalabhms added the good first issue Good for newcomers label Dec 7, 2023
@radius-triage-bot
Copy link

This issue is a great one to pickup for new contributors. It should only require small changes and not assume a deep knowledge of the Radius architecture.

We always welcome community contributions! If you would like to pick this item up sooner and submit a pull request, please visit our contribution guidelines and assign this to yourself by commenting "/assign" on this issue.

For more information on our triage process please visit our triage overview

@Gijsreyn
Copy link
Author

@vinayada1 , may this be related to #6917 ?

@vishwahiremat
Copy link
Contributor

@Gijsreyn , thats correct. Today we only allow OCI registries with anonymous pull enabled as Bicep registries for Recipes. I wonder if the registry used was configured with private access that causing this issue.

@Gijsreyn
Copy link
Author

@Gijsreyn , thats correct. Today we only allow OCI registries with anonymous pull enabled as Bicep registries for Recipes. I wonder if the registry used was configured with private access that causing this issue.

I probably think that's the case. I removed my demo, so I have to set it up again. Anyways, thanks for the reply!

@gpltaylor
Copy link
Contributor

For reference for anyone searching...

Installing Azure CLI

curl -sL https://aka.ms/InstallAzureCLIDeb | sudo bash

Login

Login to Azure using the CLI. Note that if you are using Devcontainers then you will need to use the "use-device-code" flag.

az login 
az login --use-device-code

Login ACR

Next we login to the ACR

az acr login -n <acr-name>.azurecr.io

Publish our Recipe

Next we publish out Recipe

rad bicep publish --file <bicep-file> --target "br:<acr-name>.azurecr.io/<image>:<tag>" 

rynowak added a commit that referenced this issue Jan 7, 2024
# Description

Improve error message returned when publishing to a private ACR

## 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 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: #6902 #6297

## Auto-generated summary

<!--
GitHub Copilot for docs will auto-generate a summary of the PR
-->

copilot:all

---------

Signed-off-by: Garry Taylor <[email protected]>
Co-authored-by: Ryan Nowak <[email protected]>
@rynowak rynowak reopened this Jan 7, 2024
@rynowak
Copy link
Contributor

rynowak commented Jan 7, 2024

I'm reopening this because I'm not sure that #6974 addressed it. That PR improves the error message for pushes to a registry (rad bicep publish).

@gpltaylor
Copy link
Contributor

@Gijsreyn can you confirm.
@rynowak this is resolved with the work done under "publish" IMO. The issue talk about "publish" but the command example has "show". I think this was a typo.

@rynowak
Copy link
Contributor

rynowak commented Jan 8, 2024

rad recipe publish and rad recipe show are different commands. publish pushes a Bicep module to an OCI, show shows the details of a registry that's been registered.

@Gijsreyn
Copy link
Author

@Gijsreyn can you confirm. @rynowak this is resolved with the work done under "publish" IMO. The issue talk about "publish" but the command example has "show". I think this was a typo.

Hi,

Just had some time to setup the demo again. This is what I'm trying to do:

az login
az group create --name rg-radius --location westeurope
az aks create --subscription --resource-group rg-radius --name aks-rad --node-count 1
az aks get-credentials --subscription --resource-group rg-radius --name aks-rad
az group deployment create --resource-group rg-radius --template-file oci.bicep --parameters name=acrradius
az aks update --name aks-rad --resource-group rg-radius --subscription --attach-acr acrradius
rad init
rad bicep publish --file sqlfile.bicep --target "br:acrradius.azurecr.io/azure/sql:latest"

In the oci.bicep file:

@description('Required. The Azure Container Registry')
param name string

@description('Optional. Provide a location for the registry.')
param location string = resourceGroup().location

@description('Optional. Provide a tier of your Azure Container Registry.')
param sku string = 'Basic'

resource acrResource 'Microsoft.ContainerRegistry/registries@2023-01-01-preview' = {
  name: name
  location: location
  sku: {
    name: sku
  }
  properties: {
    adminUserEnabled: false
  }
}

@description('Output the login server property for later use')
output loginServer string = acrResource.properties.loginServer

Still getting authentication required.

@gpltaylor
Copy link
Contributor

gpltaylor commented Jan 12, 2024

hi @Gijsreyn thank you for the detailed response.
Can you please ensure you have logged into the ACR. Docker and Az are not joined up at the moment.

az acr login -n acrradius

You can then Push/publish to this ACR. Let me know how you get on.
note, the latest version of the Rad CLI will be improved to show better error messaging.

@rynowak this is what I meant in my comment. The issue is with "publish" not "show"

@Gijsreyn
Copy link
Author

@gpltaylor

Successfully published Bicep file "sqlfile.bicep" to "acrradius.azurecr.io/azure/sql:latest"

No error message this time.

@gpltaylor
Copy link
Contributor

Great! Can you please #close this ticket if you're happy everything is working?

willdavsmith pushed a commit to willdavsmith/radius that referenced this issue Jan 17, 2024
…ect#6974)

# Description

Improve error message returned when publishing to a private ACR

## 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 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: radius-project#6902 radius-project#6297

## Auto-generated summary

<!--
GitHub Copilot for docs will auto-generate a summary of the PR
-->

copilot:all

---------

Signed-off-by: Garry Taylor <[email protected]>
Co-authored-by: Ryan Nowak <[email protected]>
Signed-off-by: willdavsmith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken or not working as expected good first issue Good for newcomers triaged This issue has been reviewed and triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants