Skip to content

fix(catalog/aws): a Redshift Serverless workgroup lives in its namespace on a diagram; endpoint subnets and endpoint authorizations are access, not placement #530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
avinashsana wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(catalog/aws): a Redshift Serverless workgroup lives in its namespace on a diagram; endpoint subnets and endpoint authorizations are access, not placement #530

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions ...mespace-on-a-diagram-and-a-cluster-never-lives-in-a-vpc-it-merely-authorizes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# A Redshift Serverless workgroup lives in its namespace on a diagram, and a Redshift cluster never lives in a VPC it merely authorizes

## What changed

- **`AwsRedshiftServerlessWorkgroupSpec.subnet_ids` is containment-exempt.** The namespace is the container kind (`container_kind: true`), and the workgroup is the compute plane that attaches to it -- the box AWS's own model draws around its workgroups. Until now the workgroup's subnets were also placement, so a workgroup referencing its namespace and three subnets named two rooms that are not nested in each other, and the diagram had no honest way to choose (the platform's resolver fell back to topological order, which can land the workgroup in a single subnet). Now the namespace places the workgroup and the subnets are the network it reaches into by lines -- the verdict an ECS service already carries for its subnets while its cluster places it.
- **`AwsRedshiftServerlessWorkgroupEndpointAccess.subnet_ids` is containment-exempt.** A managed VPC endpoint's subnets are the CONSUMING VPC's; the workgroup is exposed inside them, never deployed into them.
- **`AwsRedshiftClusterEndpointAuthorization.vpc_ids` is containment-exempt.** An endpoint authorization admits another account's VPCs to the cluster; the cluster never lives inside a VPC it authorizes. The cluster's own `subnet_ids` stay placement.
- The containment-decision registry (`shared/cloudresourcekind/testdata/containment_decisions.txt`) moves exactly those three lines from `contained` to `exempt`; nothing else in the registry moved.

## Why

`container_kind` says a kind is a box other resources nest inside, and `containment_exempt` says a reference into such a box is access, not placement. The workgroup carried two placement claims into two rooms that cannot nest, which is the one shape the doctrine cannot draw truthfully. Its spec already says which room is home ("a workgroup computes; the data it serves lives on the namespace it attaches to"), so the subnets are the access path and the exemption records it. The other two fields were placement by omission on references that only ever meant "let these reach me".

## How to check

```bash
go test ./shared/cloudresourcekind/... -run TestContainmentDecisions # green; the golden carries the three exempt lines
grep -n containment_exempt catalog/aws/awsredshiftserverlessworkgroup/v1alpha1/spec.proto catalog/aws/awsredshiftcluster/v1alpha1/spec.proto
```
9 changes: 6 additions & 3 deletions catalog/aws/awsredshiftcluster/v1alpha1/spec.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion catalog/aws/awsredshiftcluster/v1alpha1/spec.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -831,9 +831,13 @@ message AwsRedshiftClusterEndpointAuthorization {
// Restrict the grant to specific VPCs in the grantee account. Empty
// authorizes ALL of the account's VPCs. Reference AwsVpc vpc_id
// outputs or pass literal VPC IDs.
//
// Containment-exempt: a grant admits another VPC's endpoints to the
// cluster; the cluster never lives inside the VPC it authorizes.
repeated dev.planton.shared.foreignkey.v1.StringValueOrRef vpc_ids = 2 [
(dev.planton.shared.foreignkey.v1.default_kind) = AwsVpc,
(dev.planton.shared.foreignkey.v1.default_kind_field_path) = "status.outputs.vpc_id"
(dev.planton.shared.foreignkey.v1.default_kind_field_path) = "status.outputs.vpc_id",
(dev.planton.shared.foreignkey.v1.containment_exempt) = true
];

// Revoke the authorization on delete even if the grantee still has
Expand Down
21 changes: 15 additions & 6 deletions catalog/aws/awsredshiftserverlessworkgroup/v1alpha1/spec.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 13 additions & 2 deletions catalog/aws/awsredshiftserverlessworkgroup/v1alpha1/spec.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,16 @@ message AwsRedshiftServerlessWorkgroupSpec {
// base capacity. Empty lets AWS use the account's default VPC (only
// meaningful in accounts that still have one). Reference AwsSubnet
// subnet_id outputs or pass literal subnet IDs.
//
// Containment-exempt: on a diagram the workgroup lives inside the
// namespace it serves (the box AWS's own model draws around its
// workgroups); the subnets are where its network interfaces land, and
// the workgroup reaches into that VPC by lines -- the same verdict an
// ECS service carries for its subnets while its cluster places it.
repeated dev.planton.shared.foreignkey.v1.StringValueOrRef subnet_ids = 6 [
(dev.planton.shared.foreignkey.v1.default_kind) = AwsSubnet,
(dev.planton.shared.foreignkey.v1.default_kind_field_path) = "status.outputs.subnet_id"
(dev.planton.shared.foreignkey.v1.default_kind_field_path) = "status.outputs.subnet_id",
(dev.planton.shared.foreignkey.v1.containment_exempt) = true
];

// Security groups attached to the workgroup's endpoint. Empty uses
Expand Down Expand Up @@ -308,9 +315,13 @@ message AwsRedshiftServerlessWorkgroupEndpointAccess {
// subnet_ids (which must then be set -- CEL-enforced). Reference
// AwsSubnet subnet_id outputs or pass literal subnet IDs. Changing
// the list replaces the endpoint.
//
// Containment-exempt: these are the consuming VPC's subnets, an access
// path INTO the workgroup; the workgroup is not deployed into them.
repeated dev.planton.shared.foreignkey.v1.StringValueOrRef subnet_ids = 2 [
(dev.planton.shared.foreignkey.v1.default_kind) = AwsSubnet,
(dev.planton.shared.foreignkey.v1.default_kind_field_path) = "status.outputs.subnet_id"
(dev.planton.shared.foreignkey.v1.default_kind_field_path) = "status.outputs.subnet_id",
(dev.planton.shared.foreignkey.v1.containment_exempt) = true
];

// Security groups attached to the endpoint's network interfaces.
Expand Down
6 changes: 3 additions & 3 deletions shared/cloudresourcekind/testdata/containment_decisions.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,8 @@ contained dev.planton.aws.awsrdsinstance.v1alpha1.AwsRdsInstanceSpec.subnet_ids
contained dev.planton.aws.awsrdsproxy.v1alpha1.AwsRdsProxyEndpoint.vpc_subnet_ids -> AwsSubnet
contained dev.planton.aws.awsrdsproxy.v1alpha1.AwsRdsProxySpec.vpc_subnet_ids -> AwsSubnet
contained dev.planton.aws.awsrediselasticache.v1alpha1.AwsRedisElasticacheSpec.subnet_ids -> AwsSubnet
contained dev.planton.aws.awsredshiftcluster.v1alpha1.AwsRedshiftClusterEndpointAuthorization.vpc_ids -> AwsVpc
contained dev.planton.aws.awsredshiftcluster.v1alpha1.AwsRedshiftClusterSpec.subnet_ids -> AwsSubnet
contained dev.planton.aws.awsredshiftserverlessworkgroup.v1alpha1.AwsRedshiftServerlessWorkgroupEndpointAccess.subnet_ids -> AwsSubnet
contained dev.planton.aws.awsredshiftserverlessworkgroup.v1alpha1.AwsRedshiftServerlessWorkgroupSpec.namespace_name -> AwsRedshiftServerlessNamespace
contained dev.planton.aws.awsredshiftserverlessworkgroup.v1alpha1.AwsRedshiftServerlessWorkgroupSpec.subnet_ids -> AwsSubnet
contained dev.planton.aws.awsrestapigateway.v1alpha1.AwsRestApiGatewayAuthorizer.provider_arns -> AwsCognitoUserPool
contained dev.planton.aws.awsroute53dnsrecord.v1alpha1.AwsRoute53DnsRecordSpec.zone_id -> AwsRoute53Zone
contained dev.planton.aws.awsroute53resolverendpoint.v1alpha1.AwsRoute53ResolverEndpointIpAddress.subnet_id -> AwsSubnet
Expand Down Expand Up @@ -678,6 +675,9 @@ exempt dev.planton.aws.awslblistenerrule.v1alpha1.AwsLbListenerRuleActionAuth
exempt dev.planton.aws.awslblistenerrule.v1alpha1.AwsLbListenerRuleActionAuthenticateCognito.user_pool_domain -> AwsCognitoUserPool
exempt dev.planton.aws.awsnlb.v1alpha1.AwsNlbDns.route53_zone_id -> AwsRoute53Zone
exempt dev.planton.aws.awsopensearchdomain.v1alpha1.AwsOpenSearchDomainCognitoOptions.user_pool_id -> AwsCognitoUserPool
exempt dev.planton.aws.awsredshiftcluster.v1alpha1.AwsRedshiftClusterEndpointAuthorization.vpc_ids -> AwsVpc
exempt dev.planton.aws.awsredshiftserverlessworkgroup.v1alpha1.AwsRedshiftServerlessWorkgroupEndpointAccess.subnet_ids -> AwsSubnet
exempt dev.planton.aws.awsredshiftserverlessworkgroup.v1alpha1.AwsRedshiftServerlessWorkgroupSpec.subnet_ids -> AwsSubnet
exempt dev.planton.aws.awsroute53zone.v1alpha1.AwsRoute53ZoneVpcAssociation.vpc_id -> AwsVpc
exempt dev.planton.aws.awssagemakerdomain.v1alpha1.AwsSagemakerDomainEfsFileSystemConfig.file_system_id -> AwsElasticFileSystem
exempt dev.planton.aws.awssesconfigurationset.v1alpha1.AwsSesConfigurationSetEventDestination.event_bus -> AwsEventBridgeBus
Expand Down
Loading