Skip to content

Conversation

cotti
Copy link
Contributor

@cotti cotti commented Jul 2, 2025

Given AWSSDK's Cloudfront KeyValueStore APIs still depend on AWSSDK.Extensions.CrtIntegration, which isn't AOT-compatible, we need to fallback to an alternative method of communicating with AWS to update the Cloudfront KVS.

This PR introduces communication via CLI and extends ExternalCommandExecutor to allow customizing the amount of retries.

More information:

  • There is an issue on aws-sdk-net to discuss a new library to substitute AWSSDK.Extensions.CrtIntegration, but no implementation yet.

@cotti cotti self-assigned this Jul 2, 2025
@cotti cotti requested a review from a team as a code owner July 2, 2025 12:14
@cotti cotti added the fix label Jul 2, 2025
@cotti cotti requested a review from Copilot July 2, 2025 12:15
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR replaces direct AWS SDK calls for CloudFront KeyValueStore with CLI-based interactions to ensure AOT compatibility by removing dependencies on AWSSDK.Extensions.CrtIntegration.

  • Removed AWSSDK.CloudFront and AWSSDK.CloudFrontKeyValueStore package references.
  • Introduced JSON model types and a proxy (AwsCloudFrontKeyValueStoreProxy) that uses the AWS CLI for KVS operations.
  • Updated the deploy command to use the new proxy and enhanced retry logic in ExternalCommandExecutor.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/tooling/docs-assembler/docs-assembler.csproj Removed CloudFront SDK package references.
src/tooling/docs-assembler/Deploying/Serialization/AwsCloudFrontKeyValueStoreModels.cs Added serialization records and source-generated JSON context.
src/tooling/docs-assembler/Deploying/AwsCloudFrontKeyValueStoreProxy.cs New proxy for KVS operations using AWS CLI and batched updates.
src/tooling/docs-assembler/Cli/DeployCommands.cs Swapped SDK calls for AwsCloudFrontKeyValueStoreProxy usage.
src/tooling/Elastic.Documentation.Tooling/ExternalCommands/ExternalCommandExecutor.cs Extended Capture method with retry attempts and exposed collector.
Directory.Packages.props Removed global AWS CloudFront package versions.
Comments suppressed due to low confidence (2)

src/tooling/docs-assembler/Cli/DeployCommands.cs:138

  • [nitpick] The variable name cloudFrontClient could be misleading since this is a proxy. Consider renaming it to kvsProxy or cloudFrontKvsProxy for clarity.
		var cloudFrontClient = new AwsCloudFrontKeyValueStoreProxy(collector, new FileSystem().DirectoryInfo.New(Directory.GetCurrentDirectory()));

src/tooling/docs-assembler/Deploying/AwsCloudFrontKeyValueStoreProxy.cs:85

  • Returning [] here won’t produce a HashSet<string>. Consider returning new HashSet<string>() to match the method’s return type and avoid a compile-time error.
			return [];

@cotti cotti enabled auto-merge (squash) July 2, 2025 12:55
@cotti cotti disabled auto-merge July 2, 2025 12:55
@cotti cotti enabled auto-merge (squash) July 2, 2025 13:02
@cotti cotti merged commit d565b78 into main Jul 2, 2025
21 of 22 checks passed
@cotti cotti deleted the fix/aws_cli_redirects branch July 2, 2025 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants