Skip to content

Commit 108d95a

Browse files
Copiloticlanton
andauthored
DRY up Azure/HTTP cache providers, fix bridge plugin experiment config
Agent-Logs-Url: https://github.com/microsoft/rushstack/sessions/b57084f4-36d7-42f3-8876-7298756b887e Co-authored-by: iclanton <5010588+iclanton@users.noreply.github.com>
1 parent f9ad9e5 commit 108d95a

3 files changed

Lines changed: 77 additions & 125 deletions

File tree

rush-plugins/rush-azure-storage-build-cache-plugin/src/AzureStorageBuildCacheProvider.ts

Lines changed: 59 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -78,145 +78,83 @@ export class AzureStorageBuildCacheProvider
7878
terminal: ITerminal,
7979
cacheId: string
8080
): Promise<Buffer | undefined> {
81-
const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId, terminal);
82-
83-
try {
84-
const blobExists: boolean = await blobClient.exists();
85-
if (blobExists) {
86-
return await blobClient.downloadToBuffer();
87-
} else {
88-
return undefined;
89-
}
90-
} catch (err) {
91-
const e: IBlobError = err as IBlobError;
92-
const errorMessage: string =
93-
'Error getting cache entry from Azure Storage: ' +
94-
[e.name, e.message, e.response?.status, e.response?.parsedHeaders?.errorCode]
95-
.filter((piece: string | undefined) => piece)
96-
.join(' ');
97-
98-
if (e.response?.parsedHeaders?.errorCode === 'PublicAccessNotPermitted') {
99-
// This error means we tried to read the cache with no credentials, but credentials are required.
100-
// We'll assume that the configuration of the cache is correct and the user has to take action.
101-
terminal.writeWarningLine(
102-
`${errorMessage}\n\n` +
103-
`You need to configure Azure Storage SAS credentials to access the build cache.\n` +
104-
`Update the credentials by running "rush ${RushConstants.updateCloudCredentialsCommandName}", \n` +
105-
`or provide a SAS in the ` +
106-
`${EnvironmentVariableNames.RUSH_BUILD_CACHE_CREDENTIAL} environment variable.`
107-
);
108-
} else if (e.response?.parsedHeaders?.errorCode === 'AuthenticationFailed') {
109-
// This error means the user's credentials are incorrect, but not expired normally. They might have
110-
// gotten corrupted somehow, or revoked manually in Azure Portal.
111-
terminal.writeWarningLine(
112-
`${errorMessage}\n\n` +
113-
`Your Azure Storage SAS credentials are not valid.\n` +
114-
`Update the credentials by running "rush ${RushConstants.updateCloudCredentialsCommandName}", \n` +
115-
`or provide a SAS in the ` +
116-
`${EnvironmentVariableNames.RUSH_BUILD_CACHE_CREDENTIAL} environment variable.`
117-
);
118-
} else if (e.response?.parsedHeaders?.errorCode === 'AuthorizationPermissionMismatch') {
119-
// This error is not solvable by the user, so we'll assume it is a configuration error, and revert
120-
// to providing likely next steps on configuration. (Hopefully this error is rare for a regular
121-
// developer, more likely this error will appear while someone is configuring the cache for the
122-
// first time.)
123-
terminal.writeWarningLine(
124-
`${errorMessage}\n\n` +
125-
`Your Azure Storage SAS credentials are valid, but do not have permission to read the build cache.\n` +
126-
`Make sure you have added the role 'Storage Blob Data Reader' to the appropriate user(s) or group(s)\n` +
127-
`on your storage account in the Azure Portal.`
128-
);
129-
} else {
130-
// We don't know what went wrong, hopefully we'll print something useful.
131-
terminal.writeWarningLine(errorMessage);
132-
}
133-
return undefined;
134-
}
81+
return await this._tryGetBlobDataAsync(terminal, cacheId, async (blobClient: BlobClient) => {
82+
return await blobClient.downloadToBuffer();
83+
});
13584
}
13685

13786
public async trySetCacheEntryBufferAsync(
13887
terminal: ITerminal,
13988
cacheId: string,
14089
entryStream: Buffer
14190
): Promise<boolean> {
142-
if (!this.isCacheWriteAllowed) {
143-
terminal.writeErrorLine(
144-
'Writing to Azure Blob Storage cache is not allowed in the current configuration.'
145-
);
146-
return false;
147-
}
148-
149-
const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId, terminal);
150-
const blockBlobClient: BlockBlobClient = blobClient.getBlockBlobClient();
151-
let blobAlreadyExists: boolean = false;
152-
153-
try {
154-
blobAlreadyExists = await blockBlobClient.exists();
155-
} catch (err) {
156-
const e: IBlobError = err as IBlobError;
157-
158-
// If RUSH_BUILD_CACHE_CREDENTIAL is set but is corrupted or has been rotated
159-
// in Azure Portal, or the user's own cached credentials have been corrupted or
160-
// invalidated, we'll print the error and continue (this way we don't fail the
161-
// actual rush build).
162-
const errorMessage: string =
163-
'Error checking if cache entry exists in Azure Storage: ' +
164-
[e.name, e.message, e.response?.status, e.response?.parsedHeaders?.errorCode]
165-
.filter((piece: string | undefined) => piece)
166-
.join(' ');
167-
168-
terminal.writeWarningLine(errorMessage);
169-
}
170-
171-
if (blobAlreadyExists) {
172-
terminal.writeVerboseLine('Build cache entry blob already exists.');
173-
return true;
174-
} else {
175-
try {
176-
await blockBlobClient.upload(entryStream, entryStream.length);
177-
return true;
178-
} catch (e) {
179-
if ((e as IBlobError).statusCode === 409 /* conflict */) {
180-
// If something else has written to the blob at the same time,
181-
// it's probably a concurrent process that is attempting to write
182-
// the same cache entry. That is an effective success.
183-
terminal.writeVerboseLine(
184-
'Azure Storage returned status 409 (conflict). The cache entry has ' +
185-
`probably already been set by another builder. Code: "${(e as IBlobError).code}".`
186-
);
187-
return true;
188-
} else {
189-
terminal.writeWarningLine(`Error uploading cache entry to Azure Storage: ${e}`);
190-
return false;
191-
}
192-
}
193-
}
91+
return await this._trySetBlobDataAsync(terminal, cacheId, async (blockBlobClient: BlockBlobClient) => {
92+
await blockBlobClient.upload(entryStream, entryStream.length);
93+
});
19494
}
19595

19696
public async tryGetCacheEntryStreamByIdAsync(
19797
terminal: ITerminal,
19898
cacheId: string
19999
): Promise<NodeJS.ReadableStream | undefined> {
100+
return await this._tryGetBlobDataAsync(terminal, cacheId, async (blobClient: BlobClient) => {
101+
const downloadResponse: BlobDownloadResponseParsed = await blobClient.download();
102+
return downloadResponse.readableStreamBody;
103+
});
104+
}
105+
106+
public async trySetCacheEntryStreamAsync(
107+
terminal: ITerminal,
108+
cacheId: string,
109+
entryStream: NodeJS.ReadableStream
110+
): Promise<boolean> {
111+
return await this._trySetBlobDataAsync(
112+
terminal,
113+
cacheId,
114+
async (blockBlobClient: BlockBlobClient) => {
115+
await blockBlobClient.uploadStream(entryStream as Readable);
116+
},
117+
() => {
118+
// Drain the incoming stream since we won't consume it
119+
entryStream.resume();
120+
}
121+
);
122+
}
123+
124+
/**
125+
* Shared logic for both buffer and stream GET operations.
126+
* Checks if the blob exists, retrieves data via the provided callback, and handles errors.
127+
*/
128+
private async _tryGetBlobDataAsync<T>(
129+
terminal: ITerminal,
130+
cacheId: string,
131+
getBlobDataAsync: (blobClient: BlobClient) => Promise<T>
132+
): Promise<T | undefined> {
200133
const blobClient: BlobClient = await this._getBlobClientForCacheIdAsync(cacheId, terminal);
201134

202135
try {
203136
const blobExists: boolean = await blobClient.exists();
204137
if (blobExists) {
205-
const downloadResponse: BlobDownloadResponseParsed = await blobClient.download();
206-
return downloadResponse.readableStreamBody;
138+
return await getBlobDataAsync(blobClient);
207139
} else {
208140
return undefined;
209141
}
210142
} catch (err) {
211-
this._logBlobError(terminal, err, 'Error getting cache entry stream from Azure Storage: ');
143+
this._logBlobError(terminal, err, 'Error getting cache entry from Azure Storage: ');
212144
return undefined;
213145
}
214146
}
215147

216-
public async trySetCacheEntryStreamAsync(
148+
/**
149+
* Shared logic for both buffer and stream SET operations.
150+
* Checks write permission, whether the blob already exists, uploads via the provided callback,
151+
* and handles 409 conflict errors.
152+
*/
153+
private async _trySetBlobDataAsync(
217154
terminal: ITerminal,
218155
cacheId: string,
219-
entryStream: NodeJS.ReadableStream
156+
uploadAsync: (blockBlobClient: BlockBlobClient) => Promise<void>,
157+
onBlobAlreadyExists?: () => void
220158
): Promise<boolean> {
221159
if (!this.isCacheWriteAllowed) {
222160
terminal.writeErrorLine(
@@ -234,6 +172,10 @@ export class AzureStorageBuildCacheProvider
234172
} catch (err) {
235173
const e: IBlobError = err as IBlobError;
236174

175+
// If RUSH_BUILD_CACHE_CREDENTIAL is set but is corrupted or has been rotated
176+
// in Azure Portal, or the user's own cached credentials have been corrupted or
177+
// invalidated, we'll print the error and continue (this way we don't fail the
178+
// actual rush build).
237179
const errorMessage: string =
238180
'Error checking if cache entry exists in Azure Storage: ' +
239181
[e.name, e.message, e.response?.status, e.response?.parsedHeaders?.errorCode]
@@ -245,22 +187,24 @@ export class AzureStorageBuildCacheProvider
245187

246188
if (blobAlreadyExists) {
247189
terminal.writeVerboseLine('Build cache entry blob already exists.');
248-
// Drain the incoming stream since we won't consume it
249-
entryStream.resume();
190+
onBlobAlreadyExists?.();
250191
return true;
251192
} else {
252193
try {
253-
await blockBlobClient.uploadStream(entryStream as Readable);
194+
await uploadAsync(blockBlobClient);
254195
return true;
255196
} catch (e) {
256197
if ((e as IBlobError).statusCode === 409 /* conflict */) {
198+
// If something else has written to the blob at the same time,
199+
// it's probably a concurrent process that is attempting to write
200+
// the same cache entry. That is an effective success.
257201
terminal.writeVerboseLine(
258202
'Azure Storage returned status 409 (conflict). The cache entry has ' +
259203
`probably already been set by another builder. Code: "${(e as IBlobError).code}".`
260204
);
261205
return true;
262206
} else {
263-
terminal.writeWarningLine(`Error uploading cache entry stream to Azure Storage: ${e}`);
207+
terminal.writeWarningLine(`Error uploading cache entry to Azure Storage: ${e}`);
264208
return false;
265209
}
266210
}

rush-plugins/rush-bridge-cache-plugin/src/BridgeCachePlugin.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export class BridgeCachePlugin implements IRushPlugin {
8686
buildCacheConfiguration,
8787
rushConfiguration: {
8888
experimentsConfiguration: {
89-
configuration: { omitAppleDoubleFilesFromBuildCache }
89+
configuration: { omitAppleDoubleFilesFromBuildCache, useStreamingBuildCache }
9090
}
9191
}
9292
} = context;
@@ -120,7 +120,7 @@ export class BridgeCachePlugin implements IRushPlugin {
120120
buildCacheConfiguration,
121121
terminal,
122122
excludeAppleDoubleFiles: !!omitAppleDoubleFilesFromBuildCache,
123-
useStreamingBuildCache: false
123+
useStreamingBuildCache: !!useStreamingBuildCache
124124
}
125125
);
126126

rush-plugins/rush-http-build-cache-plugin/src/HttpBuildCacheProvider.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,10 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider {
120120
cacheId: string,
121121
objectBuffer: Buffer
122122
): Promise<boolean> {
123-
if (!this.isCacheWriteAllowed) {
124-
terminal.writeErrorLine('Writing to cache is not allowed in the current configuration.');
123+
if (!this._validateWriteAllowed(terminal, cacheId)) {
125124
return false;
126125
}
127126

128-
terminal.writeDebugLine('Uploading object with cacheId: ', cacheId);
129-
130127
try {
131128
const result: boolean | Buffer = await this._makeHttpRequestAsync({
132129
terminal: terminal,
@@ -171,13 +168,10 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider {
171168
cacheId: string,
172169
entryStream: NodeJS.ReadableStream
173170
): Promise<boolean> {
174-
if (!this.isCacheWriteAllowed) {
175-
terminal.writeErrorLine('Writing to cache is not allowed in the current configuration.');
171+
if (!this._validateWriteAllowed(terminal, cacheId)) {
176172
return false;
177173
}
178174

179-
terminal.writeDebugLine('Uploading object with cacheId: ', cacheId);
180-
181175
try {
182176
const result: IWebClientStreamResponse | false = await this._makeHttpStreamRequestAsync({
183177
terminal: terminal,
@@ -274,6 +268,20 @@ export class HttpBuildCacheProvider implements ICloudBuildCacheProvider {
274268
return this.__credentialCacheId;
275269
}
276270

271+
/**
272+
* Common validation for write operations. Returns `true` if writing is allowed,
273+
* `false` if it is not (and logs an error to the terminal).
274+
*/
275+
private _validateWriteAllowed(terminal: ITerminal, cacheId: string): boolean {
276+
if (!this.isCacheWriteAllowed) {
277+
terminal.writeErrorLine('Writing to cache is not allowed in the current configuration.');
278+
return false;
279+
}
280+
281+
terminal.writeDebugLine('Uploading object with cacheId: ', cacheId);
282+
return true;
283+
}
284+
277285
private async _makeHttpRequestAsync(options: {
278286
terminal: ITerminal;
279287
relUrl: string;

0 commit comments

Comments
 (0)