Resolve code review feedback

alerickson · alerickson · commit 63c5ea36374b · 2025-07-14T17:54:13.000-07:00
diff --git a/src/code/CredentialProvider.cs b/src/code/CredentialProvider.cs
@@ -160,13 +160,8 @@ internal static PSCredential GetCredentialsFromProvider(Uri uri, PSCmdlet cmdlet
                 if (Environment.OSVersion.Platform == PlatformID.Unix)
                 {
                     FileInfo fileInfo = new FileInfo(credProviderPath);
-                    string resolvedFilePath = Utils.GetCaseInsensitiveFilePath(fileInfo.Directory.FullName, _credProviderDll);
-                    if (resolvedFilePath != null)
-                    {
-                        credProviderPath = resolvedFilePath;
-                    }
-                    else
-                    {
+                    if (!Utils.TryGetCaseInsensitiveFilePath(fileInfo.Directory.FullName, _credProviderDll, out credProviderPath))
+                    { 
                         cmdletPassedIn.WriteError(new ErrorRecord(
                             new FileNotFoundException($"Path found '{credProviderPath}' is not a valid Azure Artifact Credential Provider executable. See https://github.com/NuGet/Home/wiki/NuGet-cross-plat-authentication-plugin#plugin-installation-and-discovery to set up the Credential Provider."),
                             "CredentialProviderFileNotFound",
diff --git a/src/code/FindHelper.cs b/src/code/FindHelper.cs
@@ -204,13 +204,7 @@ public IEnumerable<PSResourceInfo> FindByResourceName(
                 repositoryNamesToSearch.Add(currentRepository.Name);
 
                 // Set network credentials via passed in credentials, AzArtifacts CredentialProvider, or SecretManagement.
-                if (currentRepository.CredentialProvider.Equals(PSRepositoryInfo.CredentialProviderType.AzArtifacts))
-                { 
-                    _networkCredential = Utils.SetCredentialProviderNetworkCredential(currentRepository, _networkCredential, _cmdletPassedIn);
-                }
-                else {
-                    _networkCredential = Utils.SetSecretManagementNetworkCredential(currentRepository, _networkCredential, _cmdletPassedIn);
-                }
+                _networkCredential = Utils.SetNetworkCredentials(currentRepository, _networkCredential, _cmdletPassedIn);
 
                 ServerApiCall currentServer = ServerFactory.GetServer(currentRepository, _cmdletPassedIn, _networkCredential);
                 if (currentServer == null)
@@ -398,14 +392,7 @@ public IEnumerable<PSCommandResourceInfo> FindByCommandOrDscResource(
                 repositoryNamesToSearch.Add(currentRepository.Name);
 
                 // Set network credentials via passed in credentials, AzArtifacts CredentialProvider, or SecretManagement.
-                if (currentRepository.CredentialProvider.Equals(PSRepositoryInfo.CredentialProviderType.AzArtifacts))
-                {
-                    _networkCredential = Utils.SetCredentialProviderNetworkCredential(currentRepository, _networkCredential, _cmdletPassedIn);
-                }
-                else
-                {
-                    _networkCredential = Utils.SetSecretManagementNetworkCredential(currentRepository, _networkCredential, _cmdletPassedIn);
-                }
+                _networkCredential = Utils.SetNetworkCredentials(currentRepository, _networkCredential, _cmdletPassedIn);
 
                 ServerApiCall currentServer = ServerFactory.GetServer(currentRepository, _cmdletPassedIn, _networkCredential);
                 if (currentServer == null)
@@ -612,14 +599,7 @@ public IEnumerable<PSResourceInfo> FindByTag(
                 repositoryNamesToSearch.Add(currentRepository.Name);
 
                 // Set network credentials via passed in credentials, AzArtifacts CredentialProvider, or SecretManagement.
-                if (currentRepository.CredentialProvider.Equals(PSRepositoryInfo.CredentialProviderType.AzArtifacts))
-                {
-                    _networkCredential = Utils.SetCredentialProviderNetworkCredential(currentRepository, _networkCredential, _cmdletPassedIn);
-                }
-                else
-                {
-                    _networkCredential = Utils.SetSecretManagementNetworkCredential(currentRepository, _networkCredential, _cmdletPassedIn);
-                }
+                _networkCredential = Utils.SetNetworkCredentials(currentRepository, _networkCredential, _cmdletPassedIn);
 
                 ServerApiCall currentServer = ServerFactory.GetServer(currentRepository, _cmdletPassedIn, _networkCredential);
                 if (currentServer == null)
diff --git a/src/code/InstallHelper.cs b/src/code/InstallHelper.cs
@@ -287,14 +287,7 @@ private List<PSResourceInfo> ProcessRepositories(
                 sourceTrusted = currentRepository.Trusted || trustRepository;
 
                 // Set network credentials via passed in credentials, AzArtifacts CredentialProvider, or SecretManagement.
-                if (currentRepository.CredentialProvider.Equals(PSRepositoryInfo.CredentialProviderType.AzArtifacts))
-                {
-                    _networkCredential = Utils.SetCredentialProviderNetworkCredential(currentRepository, _networkCredential, _cmdletPassedIn);
-                }
-                else
-                {
-                    _networkCredential = Utils.SetSecretManagementNetworkCredential(currentRepository, _networkCredential, _cmdletPassedIn);
-                }
+                _networkCredential = Utils.SetNetworkCredentials(currentRepository, _networkCredential, _cmdletPassedIn);
 
                 ServerApiCall currentServer = ServerFactory.GetServer(currentRepository, _cmdletPassedIn, _networkCredential);
 
diff --git a/src/code/PublishHelper.cs b/src/code/PublishHelper.cs
@@ -381,14 +381,7 @@ internal void PushResource(string Repository, string modulePrefix, bool SkipDepe
                 }
 
                 // Set network credentials via passed in credentials, AzArtifacts CredentialProvider, or SecretManagement.
-                if (repository.CredentialProvider.Equals(PSRepositoryInfo.CredentialProviderType.AzArtifacts))
-                {
-                    _networkCredential = Utils.SetCredentialProviderNetworkCredential(repository, _networkCredential, _cmdletPassedIn);
-                }
-                else
-                {
-                    _networkCredential = Utils.SetSecretManagementNetworkCredential(repository, _networkCredential, _cmdletPassedIn);
-                }
+                _networkCredential = Utils.SetNetworkCredentials(repository, _networkCredential, _cmdletPassedIn);
 
                 // Check if dependencies already exist within the repo if:
                 // 1) the resource to publish has dependencies and
diff --git a/src/code/RegisterPSResourceRepository.cs b/src/code/RegisterPSResourceRepository.cs
@@ -121,13 +121,12 @@ public object GetDynamicParameters()
             // It should also not appear when using the 'Repositories' parameter set.
             if (ParameterSetName.Equals(PSGalleryParameterSet) || 
                 ParameterSetName.Equals(RepositoriesParameterSet) ||
-                Uri.EndsWith(".azurecr.io") || Uri.EndsWith(".azurecr.io/") || Uri.Contains("mcr.microsoft.com"))
+                Utils.IsContainerRegistry(Uri))
             {
                 return null;
             }
 
-            _credentialProvider = new CredentialProviderDynamicParameters();
-            return _credentialProvider;
+            return new CredentialProviderDynamicParameters();
         }
 
         #endregion
diff --git a/src/code/RepositorySettings.cs b/src/code/RepositorySettings.cs
@@ -901,7 +901,7 @@ private static APIVersion GetRepoAPIVersion(Uri repoUri)
                 // repositories with Uri Scheme "temp" may have PSPath Uri's like: "Temp:\repo" and we should consider them as local repositories.
                 return APIVersion.Local;
             }
-            else if (repoUri.AbsoluteUri.EndsWith(".azurecr.io") || repoUri.AbsoluteUri.EndsWith(".azurecr.io/") || repoUri.AbsoluteUri.Contains("mcr.microsoft.com"))
+            else if (Utils.IsContainerRegistry(repoUri.AbsoluteUri))
             {
                 return APIVersion.ContainerRegistry;
             }
diff --git a/src/code/SetPSResourceRepository.cs b/src/code/SetPSResourceRepository.cs
@@ -114,13 +114,12 @@ public object GetDynamicParameters()
             if (repository is not null && 
                 (repository.Name.Equals("PSGallery", StringComparison.OrdinalIgnoreCase) ||
                 ParameterSetName.Equals(RepositoriesParameterSet) ||
-                repository.Uri.AbsoluteUri.EndsWith(".azurecr.io") || repository.Uri.AbsoluteUri.EndsWith(".azurecr.io/") || repository.Uri.AbsoluteUri.Contains("mcr.microsoft.com")))
+                Utils.IsContainerRegistry(repository.Uri.AbsoluteUri)))
             {
                 return null;
             }
 
-            _credentialProvider = new CredentialProviderDynamicParameters();
-            return _credentialProvider;
+            return new CredentialProviderDynamicParameters();
         }
 
         #endregion
diff --git a/src/code/Utils.cs b/src/code/Utils.cs
@@ -968,6 +968,10 @@ public static NetworkCredential SetSecretManagementNetworkCredential(
             return networkCredential;
         }
 
+        #endregion
+
+        #region Credential methods
+        
         public static NetworkCredential SetCredentialProviderNetworkCredential(
             PSRepositoryInfo repository,
             NetworkCredential networkCredential,
@@ -992,6 +996,37 @@ public static NetworkCredential SetCredentialProviderNetworkCredential(
             return networkCredential;
         }
 
+        public static NetworkCredential SetNetworkCredentials(PSRepositoryInfo repository, NetworkCredential networkCredential, PSCmdlet cmdletPassedIn)
+        {
+            NetworkCredential networkCreds = new NetworkCredential();
+            if (repository.CredentialProvider.Equals(PSRepositoryInfo.CredentialProviderType.AzArtifacts))
+            {
+                cmdletPassedIn.WriteVerbose("Setting credential provider network credentials");
+                networkCreds = Utils.SetCredentialProviderNetworkCredential(repository, networkCredential, cmdletPassedIn);
+            }
+            else
+            {
+                cmdletPassedIn.WriteVerbose("Setting Secret Management network credentials");
+                networkCreds = Utils.SetSecretManagementNetworkCredential(repository, networkCredential, cmdletPassedIn);
+            }
+
+            return networkCreds;
+        }     
+
+        #endregion
+
+        #region Container Registry methods
+
+        public static bool IsContainerRegistry(string uri)
+        {
+            if (uri.EndsWith(".azurecr.io") || uri.EndsWith(".azurecr.io/") || uri.Contains("mcr.microsoft.com") || uri.StartsWith("mcr.microsoft"))
+            {
+                return true;
+            }
+
+            return false;
+        }
+        
         #endregion
 
         #region Path methods
@@ -1259,19 +1294,21 @@ internal static void GetMetadataFilesFromPath(string dirPath, string packageName
             }
         }
 
-        internal static string GetCaseInsensitiveFilePath(string directory, string fileName)
+        internal static bool TryGetCaseInsensitiveFilePath(string directory, string fileName, out string fileFound)
         {
+            fileFound = String.Empty;
             var files = Directory.GetFiles(directory);
             foreach (var file in files)
             {
                 if (string.Equals(Path.GetFileName(file), fileName, StringComparison.OrdinalIgnoreCase))
                 {
-                    return file;
+                    fileFound = file;
+                    return true;
                 }
             }
 
             // File not found
-            return null;
+            return false;
         }
 
         #endregion

Original file line number	Diff line number	Diff line change
`@@ -121,13 +121,12 @@ public object GetDynamicParameters()`
`121`	`121`	`// It should also not appear when using the 'Repositories' parameter set.`
`122`	`122`	`if (ParameterSetName.Equals(PSGalleryParameterSet) \|\|`
`123`	`123`	`ParameterSetName.Equals(RepositoriesParameterSet) \|\|`
`124`		`- Uri.EndsWith(".azurecr.io") \|\| Uri.EndsWith(".azurecr.io/") \|\| Uri.Contains("mcr.microsoft.com"))`
	`124`	`+ Utils.IsContainerRegistry(Uri))`
`125`	`125`	`{`
`126`	`126`	`return null;`
`127`	`127`	`}`
`128`	`128`
`129`		`- _credentialProvider = new CredentialProviderDynamicParameters();`
`130`		`- return _credentialProvider;`
	`129`	`+ return new CredentialProviderDynamicParameters();`
`131`	`130`	`}`
`132`	`131`
`133`	`132`	`#endregion`
Original file line number	Diff line number	Diff line change
`@@ -901,7 +901,7 @@ private static APIVersion GetRepoAPIVersion(Uri repoUri)`
`901`	`901`	`// repositories with Uri Scheme "temp" may have PSPath Uri's like: "Temp:\repo" and we should consider them as local repositories.`
`902`	`902`	`return APIVersion.Local;`
`903`	`903`	`}`
`904`		`- else if (repoUri.AbsoluteUri.EndsWith(".azurecr.io") \|\| repoUri.AbsoluteUri.EndsWith(".azurecr.io/") \|\| repoUri.AbsoluteUri.Contains("mcr.microsoft.com"))`
	`904`	`+ else if (Utils.IsContainerRegistry(repoUri.AbsoluteUri))`
`905`	`905`	`{`
`906`	`906`	`return APIVersion.ContainerRegistry;`
`907`	`907`	`}`
Original file line number	Diff line number	Diff line change
`@@ -114,13 +114,12 @@ public object GetDynamicParameters()`
`114`	`114`	`if (repository is not null &&`
`115`	`115`	`(repository.Name.Equals("PSGallery", StringComparison.OrdinalIgnoreCase) \|\|`
`116`	`116`	`ParameterSetName.Equals(RepositoriesParameterSet) \|\|`
`117`		`- repository.Uri.AbsoluteUri.EndsWith(".azurecr.io") \|\| repository.Uri.AbsoluteUri.EndsWith(".azurecr.io/") \|\| repository.Uri.AbsoluteUri.Contains("mcr.microsoft.com")))`
	`117`	`+ Utils.IsContainerRegistry(repository.Uri.AbsoluteUri)))`
`118`	`118`	`{`
`119`	`119`	`return null;`
`120`	`120`	`}`
`121`	`121`
`122`		`- _credentialProvider = new CredentialProviderDynamicParameters();`
`123`		`- return _credentialProvider;`
	`122`	`+ return new CredentialProviderDynamicParameters();`
`124`	`123`	`}`
`125`	`124`
`126`	`125`	`#endregion`