Skip to content

Add an External ID param on RAM Role Authentication #1153

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

Merged

Conversation

ihar-orca
Copy link
Contributor

@ihar-orca ihar-orca commented Dec 2, 2024

Hey guys, we want to contribute an ability to use external_id on RAM Role Authentication.
@sdk-team

@CLAassistant
Copy link

CLAassistant commented Dec 2, 2024

CLA assistant check
All committers have signed the CLA.

@ihar-orca ihar-orca changed the title CVO-17 Add an External ID param on RAM Role Authentication Add an External ID param on RAM Role Authentication Dec 2, 2024
@yndu13
Copy link

yndu13 commented Dec 6, 2024

Hi ihar-orca,

First and foremost, thank you very much for your contribution! We really appreciate the effort you've put into adding the ability to use external_id on RAM Role Authentication.

However, I noticed that there is a test case that hasn't passed. I've attached a screenshot of the error for your reference. Since you have already written some test cases, could you please take a look and update the failing one? If for any reason you are unable to make the changes, we would be happy to assist and make the updates on our end.

image

Thanks again for your valuable contribution!

Best regards,

sdk-team

@ihar-orca
Copy link
Contributor Author

hey guys @sdk-team
I fixed the test and made some other improvements and fixes

@CodeSpaceiiii
Copy link
Contributor

@ihar-orca This branch is out-of-date with the base branch, you need merge master manual.

@ihar-orca ihar-orca force-pushed the cvo-17/aliyun-cli-external-id branch 2 times, most recently from 352f3bf to 080c99b Compare May 7, 2025 15:34
@ihar-orca ihar-orca force-pushed the cvo-17/aliyun-cli-external-id branch from 080c99b to c25a1d2 Compare May 7, 2025 15:39
@ihar-orca
Copy link
Contributor Author

hey @sdk-team @CodeSpaceiiii guys I resolved conflicts and updated the base branch

@codecov-commenter
Copy link

codecov-commenter commented May 8, 2025

Codecov Report

Attention: Patch coverage is 87.50000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 83.56%. Comparing base (0f9028d) to head (b5e1bd5).

Files with missing lines Patch % Lines
config/configure_list.go 50.00% 3 Missing ⚠️
config/configure_set.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1153      +/-   ##
==========================================
+ Coverage   83.54%   83.56%   +0.02%     
==========================================
  Files          48       48              
  Lines        4618     4650      +32     
==========================================
+ Hits         3858     3886      +28     
- Misses        589      593       +4     
  Partials      171      171              

☔ 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.

@CodeSpaceiiii CodeSpaceiiii merged commit 6385e63 into aliyun:master May 8, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants