Remove Build command requirement for service principal #1293
Conversation
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
lbussell
force-pushed
the
dev/loganbussell/build
branch
from
c8d9e0b to
9cf0fb0
Compare
src/Microsoft.DotNet.ImageBuilder/src/Commands/GetStaleImagesOptions.cs
Outdated
Show resolved
Hide resolved
| }) | ||
| }; | ||
| }), | ||
| CreateOption<string?>("tenant", nameof(RegistryCredentialsOptions.Tenant), |
There was a problem hiding this comment.
Can't we just derive this value from the tenant environment variable just as DefaultAzureCredential is doing? I'm thinking that GetDefaultAccessTokenAsync could return both the token and the tenant. Or is there an API that can get the tenant from the token?
There was a problem hiding this comment.
Good idea. I'll work on this.
| _inner.GetManifestDigestShaAsync(tag, isDryRun); | ||
| } | ||
|
|
||
| internal class InnerManifestService : IInnerManifestService |
There was a problem hiding this comment.
It seems strange that all these other interfaces and classes are contained in this file but not IInnerManifestService. They should either all be in separate files or all contained in this file.
lbussell
force-pushed
the
dev/loganbussell/build
branch
from
a9e1324 to
354896e
Compare
|
The force push contains no changes, I just did a rebase instead of a merge by accident. I ran a sanity check of the build command and it seems OK - [internal link] - another one without Windows so it can run publishing commands We should merge to get the actual ImageBuilder image built and continue validating. We can address #1296 at a later date. |
Sorry this PR is so large!
Related: #1264
Notes:
DockerServicenever needs auth,ManifestServiceuses auth when we have it, therefore I've separated the two classes out so thatDockerServiceis never concerned about auth.ManifestService(nowInnerManifestService) so extension methods can be mocked for testing (DockerServicepreviously served this function).ManifestServiceFactoryto handle passing auth to theManifestServicein one place.RegistryCredentialsProviderwhich uses logic similar toRegistryContentClientFactoryto dynamically get username/password fordocker login.TODO:
RegistryContentClientFactoryTests are failing