Skip to content

Conversation

@yrobla
Copy link
Contributor

@yrobla yrobla commented Nov 26, 2025

No description provided.

@yrobla yrobla requested review from JAORMX, Copilot and jhrozek November 26, 2025 14:45
@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Nov 26, 2025
Copy link
Contributor

Copilot AI left a 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 simplifies the header injection authentication converter by removing the intermediate header_value_env placeholder field. Previously, the converter would set this placeholder during metadata conversion and later remove it during secret resolution. The new implementation is cleaner: ConvertToMetadata only returns the header_name, and ResolveSecrets directly adds the header_value with the actual secret.

  • Removed unnecessary header_value_env placeholder field from the conversion flow
  • Updated tests to reflect the simplified metadata structure
  • Improved comments to better explain the two-phase conversion process

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/vmcp/auth/converters/header_injection.go Removed setting and deletion of header_value_env placeholder; updated comments to clarify that ConvertToMetadata returns minimal metadata and ResolveSecrets adds the secret value
pkg/vmcp/auth/converters/header_injection_test.go Updated test expectations to verify only header_name is present after ConvertToMetadata, and both fields are present after ResolveSecrets

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yrobla yrobla force-pushed the fix/remove_header_value_env branch from c259562 to cdee526 Compare November 26, 2025 14:50
@github-actions github-actions bot added size/XS Extra small PR: < 100 lines changed and removed size/XS Extra small PR: < 100 lines changed labels Nov 26, 2025
@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.96%. Comparing base (3e4fd23) to head (cdee526).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2744   +/-   ##
=======================================
  Coverage   55.96%   55.96%           
=======================================
  Files         318      318           
  Lines       30677    30675    -2     
=======================================
  Hits        17168    17168           
+ Misses      12022    12020    -2     
  Partials     1487     1487           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yrobla yrobla merged commit e66e418 into main Nov 26, 2025
33 checks passed
@yrobla yrobla deleted the fix/remove_header_value_env branch November 26, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants