-
Notifications
You must be signed in to change notification settings - Fork 120
Improve oci error message #10816
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
base: main
Are you sure you want to change the base?
Improve oci error message #10816
Conversation
Co-authored-by: brooke-hamilton <[email protected]>
Co-authored-by: brooke-hamilton <[email protected]>
Signed-off-by: Brooke Hamilton <[email protected]>
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.
Pull Request Overview
This PR enhances error messages when users provide invalid OCI repository names in the rad bicep publish --target argument. The changes wrap low-level oras-go validation errors with detailed, user-friendly messages that explain OCI naming rules and provide concrete examples.
Key changes:
- Added
enhanceOCIError()function to detect and enhance different types of OCI validation errors (repository, registry, tag, and format errors) - Updated
extractDestination()to call the new error enhancement function - Added comprehensive test suite covering uppercase repository names, invalid tags, missing components, and valid inputs
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/cli/cmd/bicep/publish/publish.go | Implements enhanceOCIError() function that wraps oras-go errors with detailed OCI naming guidance and examples |
| pkg/cli/cmd/bicep/publish/publish_test.go | Adds TestRunner_extractDestination_EnhancedErrors test suite with 6 test cases validating error message content for various invalid and valid OCI targets |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10816 +/- ##
==========================================
+ Coverage 50.43% 50.45% +0.01%
==========================================
Files 672 672
Lines 42057 42065 +8
==========================================
+ Hits 21213 21222 +9
Misses 18788 18788
+ Partials 2056 2055 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@zachcasper please review the error messages. |
|
Two thoughts:
My suggestion would be to change the message to: I think be saying the value must be a hostname is clear enough and the naming rules are not needed. |
Signed-off-by: Brooke Hamilton <[email protected]>
@zachcasper these issues are addressed now. You can run the script below to see the error messages. Below the script is a sample of the output, which shows the error messages in each case. Please review the messages and provide feedback--I'm happy to make more revisions.
Test Script#!/bin/bash
# ============================================================================
# Test Script for Enhanced OCI Error Messages in rad bicep publish
# ============================================================================
# This script tests the improved error messages for invalid OCI references
# in the 'rad bicep publish' command.
#
# Prerequisites:
# - rad CLI built from the copilot/improve-oci-error-message branch
# - A simple Bicep file to publish
# ============================================================================
set -euo pipefail
echo "============================================================================"
echo "Testing Enhanced OCI Error Messages for 'rad bicep publish'"
echo "============================================================================"
echo ""
# Create a simple test Bicep file
TEST_BICEP_FILE="/tmp/test-oci-errors.bicep"
cat > "$TEST_BICEP_FILE" << 'EOF'
// Simple test Bicep file
metadata description = 'Test file for OCI error message testing'
@description('Test parameter')
param testParam string = 'default'
output result string = testParam
EOF
echo "Created test Bicep file: $TEST_BICEP_FILE"
echo ""
# Path to rad CLI - adjust if needed
RAD_CLI="./dist/linux_amd64/release/rad"
# Check if rad CLI exists
if [[ ! -f "$RAD_CLI" ]]; then
echo "rad CLI not found at $RAD_CLI"
echo "Building rad CLI..."
make build
fi
echo "============================================================================"
echo "Test 1: Missing 'br:' prefix"
echo "============================================================================"
echo "Command: $RAD_CLI bicep publish --file $TEST_BICEP_FILE --target localhost:5000/test/module:v1"
echo ""
$RAD_CLI bicep publish --file "$TEST_BICEP_FILE" --target "localhost:5000/test/module:v1" 2>&1 || true
echo ""
echo "============================================================================"
echo "Test 2: Uppercase letters in repository name (invalid)"
echo "============================================================================"
echo "Command: $RAD_CLI bicep publish --file $TEST_BICEP_FILE --target br:localhost:5000/MyRegistry/Data/mySqlDatabases:latest"
echo ""
$RAD_CLI bicep publish --file "$TEST_BICEP_FILE" --target "br:localhost:5000/MyRegistry/Data/mySqlDatabases:latest" 2>&1 || true
echo ""
echo "============================================================================"
echo "Test 3: Uppercase at start of repository path"
echo "============================================================================"
echo "Command: $RAD_CLI bicep publish --file $TEST_BICEP_FILE --target br:localhost:5000/TestRepo/module:v1"
echo ""
$RAD_CLI bicep publish --file "$TEST_BICEP_FILE" --target "br:localhost:5000/TestRepo/module:v1" 2>&1 || true
echo ""
echo "============================================================================"
echo "Test 4: Invalid tag starting with hyphen"
echo "============================================================================"
echo "Command: $RAD_CLI bicep publish --file $TEST_BICEP_FILE --target br:localhost:5000/myregistry/data:-invalid"
echo ""
$RAD_CLI bicep publish --file "$TEST_BICEP_FILE" --target "br:localhost:5000/myregistry/data:-invalid" 2>&1 || true
echo ""
echo "============================================================================"
echo "Test 5: Missing repository (only registry)"
echo "============================================================================"
echo "Command: $RAD_CLI bicep publish --file $TEST_BICEP_FILE --target br:localhost:5000"
echo ""
$RAD_CLI bicep publish --file "$TEST_BICEP_FILE" --target "br:localhost:5000" 2>&1 || true
echo ""
echo "============================================================================"
echo "Test 6: Valid format (will fail at registry connection, but should pass validation)"
echo "============================================================================"
echo "Command: $RAD_CLI bicep publish --file $TEST_BICEP_FILE --target br:localhost:5000/myregistry/data/module:latest --plain-http"
echo ""
$RAD_CLI bicep publish --file "$TEST_BICEP_FILE" --target "br:localhost:5000/myregistry/data/module:latest" --plain-http 2>&1 || true
echo ""
echo "============================================================================"
echo "Test Complete"
echo "============================================================================"
echo ""
echo "Expected behavior:"
echo "- Tests 1-5 should show enhanced error messages with:"
echo " - The invalid target displayed WITH the 'br:' prefix"
echo " - Help text: \"The target must be a valid Bicep OCI registry reference in the form 'br:<OCI-registry-hostname>/<module-path>:<tag>'.\""
echo " - The underlying cause from oras-go"
echo ""
echo "- Test 6 should fail with a connection error (no registry running),"
echo " NOT a validation error, proving the valid format passes validation."
echo ""
# Cleanup
rm -f "$TEST_BICEP_FILE"
echo "Cleaned up test file."Script Output |
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
LGTM except test 1 seems to be an outlier compared to the others. It's accurate but test 2-5 are better. |
Co-authored-by: Copilot <[email protected]> Signed-off-by: Brooke Hamilton <[email protected]>
Signed-off-by: Brooke Hamilton <[email protected]>
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
Description
When users provide invalid OCI repository names in
rad bicep publish --target, the error from oras-go (invalid reference: invalid repository "...") doesn't explain OCI naming rules or how to fix the issue.Changes
enhanceOCIError()function that wraps oras-go validation errors with detailed messages explaining:extractDestination()to use enhanced error handlingExample
Before:
After:
Type of change
Fixes: #10517
Contributor checklist
Please verify that the PR meets the following requirements, where applicable:
Original prompt
rad bicep publish--target argument #10517