Skip to content

Conversation

@rswanson
Copy link
Member

No description provided.

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@rswanson rswanson force-pushed the swanny/builder-component branch from b7c83d9 to 5232267 Compare May 15, 2025 17:40
@rswanson rswanson requested a review from Copilot May 15, 2025 17:41
Copy link
Contributor

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 introduces a new Pulumi “builder” component for deploying a builder service to Kubernetes, including argument validation, environment‐variable mapping, and resource orchestration.

  • Add go.mod for the new module and pull in required dependencies
  • Implement BuilderComponentArgs and BuilderEnv validation logic
  • Define core types, helpers for KMS policy and env‐var generation, and the NewBuilder function to wire up ServiceAccount, IAM roles/policies, Deployment, Service, and PodMonitor

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
go.mod Declare new module signet-infra-components and its dependencies
builder/validation.go Add Validate() methods for component args and environment struct
builder/types.go Define BuilderComponentArgs, BuilderEnv, interfaces, and policies
builder/helpers.go Generate KMS policy JSON and convert BuilderEnv to EnvVarArray
builder/builder.go Implement NewBuilder to deploy all Kubernetes and IAM resources
Comments suppressed due to low confidence (4)

builder/builder.go:85

  • [nitpick] The logical name quinceyAppPolicy doesn’t match the builder context. Rename it to something like builder-kms-policy or include args.Name for consistency.
policy, err := iam.NewPolicy(ctx, "quinceyAppPolicy", &iam.PolicyArgs{

builder/builder.go:204

  • [nitpick] Logical name builder-svcmon differs from metadata name builder-pod-monitor. Align these names to avoid confusion during resource diffs.
_, err = crd.NewCustomResource(ctx, "builder-svcmon", &crd.CustomResourceArgs{

builder/types.go:63

  • [nitpick] Field prefixes mix Oauth and OAuth. Standardize the casing across all fields (e.g. always use OAuth) to improve consistency.
OAuthClientId            pulumi.StringInput `pulumi:"oauthClientId"`

builder/validation.go:8

  • No unit tests were added for validation logic. Consider adding tests to cover both success and failure paths in Validate().
func (args *BuilderComponentArgs) Validate() error {

@rswanson rswanson marked this pull request as ready for review May 15, 2025 18:49
@rswanson rswanson self-assigned this May 16, 2025
@rswanson rswanson merged commit 50dd282 into main May 19, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants