parse image pull auth from env #1382

sctb512 · 2023-07-24T09:42:02Z

Details

This commit attempts to avoid loading auth from the nydusd configuration file, which is insecure for users. The auth loaded from env will overwrite the auth loaded from file if it is not null.

Types of changes

What types of changes does your PullRequest introduce? Put an x in all the boxes that apply:

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation Update (if none of the other choices apply)

Checklist

Go over all the following points, and put an x in all the boxes that apply.

I have updated the documentation accordingly.
I have added tests to cover my changes.

anolis-bot · 2023-07-24T09:42:11Z

@sctb512 , a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/86636

codecov · 2023-07-24T09:46:48Z

Codecov Report

Merging #1382 (db0c32a) into master (0c01dac) will increase coverage by 0.05%.
The diff coverage is 82.75%.

@@            Coverage Diff             @@
##           master    #1382      +/-   ##
==========================================
+ Coverage   46.27%   46.33%   +0.05%     
==========================================
  Files         122      122              
  Lines       37975    38032      +57     
  Branches    37975    38032      +57     
==========================================
+ Hits        17574    17621      +47     
- Misses      19478    19487       +9     
- Partials      923      924       +1

Files Changed	Coverage Δ
src/bin/nydusd/main.rs	`0.19% <0.00%> (-0.01%)`	⬇️
api/src/config.rs	`71.43% <94.11%> (+0.91%)`	⬆️

... and 2 files with indirect coverage changes

anolis-bot · 2023-07-24T09:54:57Z

@sctb512 , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

src/bin/nydusd/main.rs

jiangliu · 2023-07-25T06:18:59Z

There's a PR to enable passing nydusd configuration information through pipe instead of a file, seems these two PRs are both to enhance the security of auth info. So what's the relationship between the two PRs?

#1379

sctb512 · 2023-07-25T06:52:36Z

There's a PR to enable passing nydusd configuration information through pipe instead of a file, seems these two PRs are both to enhance the security of auth info. So what's the relationship between the two PRs?

The PR #1379 attempts to propose an interface for nydus-snapshotter to update configuration (including auth). This PR avoids loading auth from disk when starting nydusd. I think they are not in conflict with each other.

In short, we can parse auth from the environment variable and update it via pipe.

anolis-bot · 2023-07-25T06:55:36Z

@sctb512 , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/86777

anolis-bot · 2023-07-25T07:08:22Z

@sctb512 , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

api/src/config.rs

anolis-bot · 2023-07-26T02:38:11Z

@sctb512 , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/86968

jiangliu · 2023-07-26T03:11:01Z

api/src/http.rs

@@ -31,6 +31,8 @@ pub struct ApiMountCmd {
    pub fs_type: String,
    /// Configuration for the filesystem.
    pub config: String,
+    /// Authorization for private registry.
+    pub auth: Option<String>,


Why is this extension needed? I think callers should merge the auth info into config.

Yes, you are correct. Fixed it.

anolis-bot · 2023-07-26T06:09:43Z

@sctb512 , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/87003

anolis-bot · 2023-07-26T06:14:52Z

@sctb512 , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/87006

anolis-bot · 2023-07-26T08:29:05Z

@sctb512 , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

anolis-bot · 2023-07-26T08:41:31Z

@sctb512 , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

anolis-bot · 2023-07-26T08:53:09Z

@sctb512 , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

adamqqqplay

Maybe we can prefix the first commit log with nydusd? nydusd: parse image pull auth from env

Signed-off-by: Bin Tang <[email protected]>

anolis-bot · 2023-07-27T02:05:23Z

@sctb512 , the code has been updated, so a new test job has been submitted. Please wait in patience. The test job url: https://tone.openanolis.cn/ws/nrh4nnio/test_result/87150

sctb512 · 2023-07-27T02:07:24Z

Maybe we can prefix the first commit log with nydusd? nydusd: parse image pull auth from env

Done.

adamqqqplay

LGTM, Thanks!

anolis-bot · 2023-07-27T02:24:53Z

@sctb512 , The CI test is completed, please check result:

Test Case		Test Result
build rust golang image		✅ SUCCESS
compile nydusd		✅ SUCCESS
compile ctr remote		✅ SUCCESS
compile nydus snapshotter		✅ SUCCESS
run container with rafs		✅ SUCCESS
run container with zran		✅ SUCCESS
run container with rafs and compile linux		✅ SUCCESS

Congratulations, your test job passed!

changweige

LGTM, thanks

sctb512 requested a review from a team as a code owner July 24, 2023 09:42

sctb512 requested review from liubin, luodw and adamqqqplay and removed request for a team July 24, 2023 09:42

anolis-bot added the anolis_testing label Jul 24, 2023

anolis-bot added anolis_test_pass and removed anolis_testing labels Jul 24, 2023

sctb512 mentioned this pull request Jul 24, 2023

Store auth in keyring and transfer it to nydusd via environment variable containerd/nydus-snapshotter#512

Open

jiangliu reviewed Jul 25, 2023

View reviewed changes

src/bin/nydusd/main.rs Outdated Show resolved Hide resolved

sctb512 force-pushed the get-auth-from-env branch from cce27cc to f74f0fa Compare July 25, 2023 06:55

anolis-bot added anolis_testing and removed anolis_test_pass labels Jul 25, 2023

anolis-bot added anolis_test_pass and removed anolis_testing labels Jul 25, 2023

sctb512 mentioned this pull request Jul 25, 2023

Backport/v2.2 parse image pull auth from env #1383

Merged

6 tasks

jiangliu reviewed Jul 25, 2023

View reviewed changes

api/src/config.rs Outdated Show resolved Hide resolved

sctb512 force-pushed the get-auth-from-env branch from f74f0fa to b387c7f Compare July 26, 2023 02:38

anolis-bot added anolis_testing and removed anolis_test_pass labels Jul 26, 2023

jiangliu reviewed Jul 26, 2023

View reviewed changes

sctb512 force-pushed the get-auth-from-env branch from b387c7f to e272663 Compare July 26, 2023 06:09

sctb512 force-pushed the get-auth-from-env branch from e272663 to be5a0e2 Compare July 26, 2023 06:14

anolis-bot added anolis_test_pass and removed anolis_testing labels Jul 26, 2023

jiangliu approved these changes Jul 27, 2023

View reviewed changes

adamqqqplay reviewed Jul 27, 2023

View reviewed changes

sctb512 added 3 commits July 27, 2023 10:04

nydusd: parse image pull auth from env

3c38398

Signed-off-by: Bin Tang <[email protected]>

docs: introduce IMAGE_PULL_AUTH env

ad50ffa

Signed-off-by: Bin Tang <[email protected]>

fs: add test for filling auth

db0c32a

Signed-off-by: Bin Tang <[email protected]>

sctb512 force-pushed the get-auth-from-env branch from be5a0e2 to db0c32a Compare July 27, 2023 02:05

anolis-bot added anolis_testing and removed anolis_test_pass labels Jul 27, 2023

adamqqqplay approved these changes Jul 27, 2023

View reviewed changes

anolis-bot added anolis_test_pass and removed anolis_testing labels Jul 27, 2023

changweige reviewed Jul 27, 2023

View reviewed changes

changweige approved these changes Jul 27, 2023

View reviewed changes

changweige merged commit 82ebd11 into dragonflyoss:master Jul 27, 2023
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

parse image pull auth from env #1382

parse image pull auth from env #1382

sctb512 commented Jul 24, 2023

anolis-bot commented Jul 24, 2023

codecov bot commented Jul 24, 2023 •

edited

Loading

anolis-bot commented Jul 24, 2023

jiangliu commented Jul 25, 2023 •

edited

Loading

sctb512 commented Jul 25, 2023

anolis-bot commented Jul 25, 2023

anolis-bot commented Jul 25, 2023

anolis-bot commented Jul 26, 2023

jiangliu Jul 26, 2023

sctb512 Jul 26, 2023 •

edited

Loading

anolis-bot commented Jul 26, 2023

anolis-bot commented Jul 26, 2023

anolis-bot commented Jul 26, 2023

anolis-bot commented Jul 26, 2023

anolis-bot commented Jul 26, 2023

adamqqqplay left a comment

anolis-bot commented Jul 27, 2023

sctb512 commented Jul 27, 2023

adamqqqplay left a comment

anolis-bot commented Jul 27, 2023

changweige left a comment

parse image pull auth from env #1382

parse image pull auth from env #1382

Conversation

sctb512 commented Jul 24, 2023

Details

Types of changes

Checklist

anolis-bot commented Jul 24, 2023

codecov bot commented Jul 24, 2023 • edited Loading

Codecov Report

anolis-bot commented Jul 24, 2023

jiangliu commented Jul 25, 2023 • edited Loading

sctb512 commented Jul 25, 2023

anolis-bot commented Jul 25, 2023

anolis-bot commented Jul 25, 2023

anolis-bot commented Jul 26, 2023

jiangliu Jul 26, 2023

Choose a reason for hiding this comment

sctb512 Jul 26, 2023 • edited Loading

Choose a reason for hiding this comment

anolis-bot commented Jul 26, 2023

anolis-bot commented Jul 26, 2023

anolis-bot commented Jul 26, 2023

anolis-bot commented Jul 26, 2023

anolis-bot commented Jul 26, 2023

adamqqqplay left a comment

Choose a reason for hiding this comment

anolis-bot commented Jul 27, 2023

sctb512 commented Jul 27, 2023

adamqqqplay left a comment

Choose a reason for hiding this comment

anolis-bot commented Jul 27, 2023

changweige left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 24, 2023 •

edited

Loading

jiangliu commented Jul 25, 2023 •

edited

Loading

sctb512 Jul 26, 2023 •

edited

Loading