Container v1 support

[John-Leitch]

As discussed in #264 and #300, Aspire now generates container.v1 resources in cases that were previously dockerfile.v0. This PR adds fairly complete support for container.v1, reusing existing logic container.v0 and dockerfile.v0 depending on whether an image or build is specified. Only two container.v1 properties are unsupported: deployment and build.secrets, with the former appearing to be Azure specific and potentially irrelevant.

[qodo-code-review]

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 264 - PR Code Verified Compliant requirements: • Support for container.v1 resources generated by Aspire when using WithDockerfile • Handle the new build entry in docker-compose Requires further human verification: • Ensure compatibility with Kubernetes manifests 300 - Fully compliant Compliant requirements: • Fix crash when Aspir8 tries to read manifest.json it just generated with container.v1 resources • Handle the build section in the manifest.json properly • Support containers modified with a Dockerfile in Aspire

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The GetImageFromContainerResource method might throw an exception if a ContainerV1Resource has neither Image nor Build properties set. Consider adding validation or more graceful error handling. private string GetImageFromContainerResource(KeyValuePair<string, Resource> resource) { switch (resource.Value) { case ContainerResource containerV0: return containerV0.Image; case ContainerV1Resource containerV1: if (containerV1.Image != null) { return containerV1.Image; } else if (containerV1.Build != null) { return GetCachedImages(resource.Key).First(); } else { throw new InvalidOperationException($"{AspireComponentLiterals.ContainerV1} must have image or build property."); } default: throw new InvalidOperationException($"Unexpected resource type {resource.Value?.GetType().Name}"); } } Null Reference Risk The CreateComposeEntry method assumes container is not null when casting to specific types. Consider adding null checks to prevent potential runtime exceptions. public override ComposeService CreateComposeEntry(CreateComposeEntryOptions options) { var response = new ComposeService(); var container = options.Resource.Value as TContainerResource; var service = Builder.MakeService(options.Resource.Key); if (container is ContainerResource containerV0) { service.WithImage(containerV0.Image.ToLowerInvariant()); } else if (container is ContainerV1Resource containerV1) { if (containerV1.Image != null) { service.WithImage(containerV1.Image.ToLowerInvariant()); } else if (containerV1.Build != null && options.ComposeBuilds == true) { service.WithBuild(builder => builder .WithContext(_fileSystem.GetFullPath(containerV1.Build.Context)) .WithDockerfile(_fileSystem.GetFullPath(containerV1.Build.Dockerfile)) .Build()); } } Resource Selection Logic The SelectComposeItemsToIncludeAsComposeBuilds method now filters resources differently, which might affect which resources are included in the build process. Verify this change doesn't exclude valid resources. var dockerFileEntries = CurrentState.LoadedAspireManifestResources .Where(x => x.Value is DockerfileResource || (x.Value is ContainerV1Resource c && c.Build != null)) .Select(x => x.Key) .ToList();

[qodo-code-review]

Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

PR Code Suggestions ✨

No code suggestions found for the PR.

[prom3theu5]

Awesome - thanks John, looks good and will close off the issues that have started trickling in related to the v1 resource
I'd completely missed that that had been implemented