Fix an issue with writing to Azure storage

microsoft · Dec 29, 2020 · 4037910 · 4037910
1 parent f736789
commit 4037910
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 21 deletions.
diff --git a/apps/rush-lib/src/logic/buildCache/AzureStorageBuildCacheProvider.ts b/apps/rush-lib/src/logic/buildCache/AzureStorageBuildCacheProvider.ts
@@ -18,14 +18,11 @@ import { EnvironmentConfiguration, EnvironmentVariableNames } from '../../api/En
 import { CredentialCache, ICredentialCacheEntry } from '../CredentialCache';
 import { RushConstants } from '../RushConstants';
 import { Utilities } from '../../utilities/Utilities';
-import {
-  CloudBuildCacheProviderBase,
-  ICloudBuildCacheProviderBaseOptions
-} from './CloudBuildCacheProviderBase';
+import { CloudBuildCacheProviderBase } from './CloudBuildCacheProviderBase';
 
 export type AzureEnvironmentNames = keyof typeof AzureAuthorityHosts;
 
-export interface IAzureStorageBuildCacheProviderOptions extends ICloudBuildCacheProviderBaseOptions {
+export interface IAzureStorageBuildCacheProviderOptions {
   storageContainerName: string;
   storageAccountName: string;
   azureEnvironment?: AzureEnvironmentNames;
@@ -40,18 +37,19 @@ export class AzureStorageBuildCacheProvider extends CloudBuildCacheProviderBase
   private readonly _storageContainerName: string;
   private readonly _azureEnvironment: AzureEnvironmentNames;
   private readonly _blobPrefix: string | undefined;
-  private readonly _isCacheWriteAllowed: boolean;
   private __credentialCacheId: string | undefined;
 
+  public readonly isCacheWriteAllowed: boolean;
+
   private _containerClient: ContainerClient | undefined;
 
   public constructor(options: IAzureStorageBuildCacheProviderOptions) {
-    super(options);
+    super();
     this._storageAccountName = options.storageAccountName;
     this._storageContainerName = options.storageContainerName;
     this._azureEnvironment = options.azureEnvironment || 'AzurePublicCloud';
     this._blobPrefix = options.blobPrefix;
-    this._isCacheWriteAllowed = options.isCacheWriteAllowed;
+    this.isCacheWriteAllowed = options.isCacheWriteAllowed;
 
     if (!(this._azureEnvironment in AzureAuthorityHosts)) {
       throw new Error(
@@ -70,7 +68,7 @@ export class AzureStorageBuildCacheProvider extends CloudBuildCacheProviderBase
         this._storageContainerName
       ];
 
-      if (this._isCacheWriteAllowed) {
+      if (this.isCacheWriteAllowed) {
         cacheIdParts.push('cacheWriteAllowed');
       }
 
@@ -107,6 +105,13 @@ export class AzureStorageBuildCacheProvider extends CloudBuildCacheProviderBase
     cacheId: string,
     entryStream: Buffer
   ): Promise<boolean> {
+    if (!this.isCacheWriteAllowed) {
+      terminal.writeErrorLine(
+        'Writing to Azure Blob Storage cache is not allowed in the current configuration.'
+      );
+      return false;
+    }
+
     const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId);
     const blockBlobClient: BlockBlobClient = blobClient.getBlockBlobClient();
     try {
@@ -189,9 +194,12 @@ export class AzureStorageBuildCacheProvider extends CloudBuildCacheProviderBase
       }
 
       let blobServiceClient: BlobServiceClient;
-      if (sasString || !this._isCacheWriteAllowed) {
+      if (sasString) {
         const connectionString: string = this._getConnectionString(sasString);
         blobServiceClient = BlobServiceClient.fromConnectionString(connectionString);
+      } else if (!this.isCacheWriteAllowed) {
+        // If cache write isn't allowed and we don't have a credential, assume the blob supports anonymous read
+        blobServiceClient = new BlobServiceClient(this._storageAccountUrl);
       } else {
         throw new Error(
           "An Azure Storage SAS credential hasn't been provided, or has expired. " +
@@ -235,7 +243,7 @@ export class AzureStorageBuildCacheProvider extends CloudBuildCacheProviderBase
 
     const containerSasPermissions: ContainerSASPermissions = new ContainerSASPermissions();
     containerSasPermissions.read = true;
-    containerSasPermissions.write = this._isCacheWriteAllowed;
+    containerSasPermissions.write = this.isCacheWriteAllowed;
 
     const queryParameters: SASQueryParameters = generateBlobSASQueryParameters(
       {

diff --git a/apps/rush-lib/src/logic/buildCache/CloudBuildCacheProviderBase.ts b/apps/rush-lib/src/logic/buildCache/CloudBuildCacheProviderBase.ts
@@ -3,10 +3,8 @@
 
 import { Terminal } from '@rushstack/node-core-library';
 
-export interface ICloudBuildCacheProviderBaseOptions {}
-
 export abstract class CloudBuildCacheProviderBase {
-  public constructor(options: ICloudBuildCacheProviderBaseOptions) {}
+  public abstract readonly isCacheWriteAllowed: boolean;
 
   public abstract tryGetCacheEntryBufferByIdAsync(
     terminal: Terminal,

diff --git a/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts b/apps/rush-lib/src/logic/buildCache/ProjectBuildCache.ts
@@ -302,13 +302,10 @@ export class ProjectBuildCache {
       cacheEntryBuffer
     );
 
-    const setCloudCacheEntryPromise:
-      | Promise<boolean>
-      | undefined = this._cloudBuildCacheProvider?.trySetCacheEntryBufferAsync(
-      this._terminal,
-      cacheId,
-      cacheEntryBuffer
-    );
+    const setCloudCacheEntryPromise: Promise<boolean> | undefined =
+      this._cloudBuildCacheProvider?.isCacheWriteAllowed === true
+        ? this._cloudBuildCacheProvider.trySetCacheEntryBufferAsync(this._terminal, cacheId, cacheEntryBuffer)
+        : undefined;
 
     let updateLocalCacheSuccess: boolean;
     let updateCloudCacheSuccess: boolean;