Skip to content

Commit

Permalink
Fail fast on schema lookup failures (#1357)
Browse files Browse the repository at this point in the history
Presently, if an input program is ill-formed (e.g. refers to attributes
that do not appear in the schema), we will generate an ill-formed output
program instead of failing with an error. Specifically we will generate
Java code containing `PANIC`s where we fail to dereference `nil`s picked
up as a result of failed schema lookups. This commit fixes this
behaviour so that Java code generation proceeds as we do for other
languages -- validating schema lookups and failing outright instead of
generating ill-formed programs.

* Fixes #1346 
* Fixes #1347 
* Fixes #1348
* Fixes #1350 
* Fixes #1351 
* Fixes #1352 
* Fixes #1353
* Fixes #1355
  • Loading branch information
lunaris committed May 8, 2024
2 parents c8db4b9 + 5f3ecb0 commit b69848a
Show file tree
Hide file tree
Showing 18 changed files with 70 additions and 54 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
### Improvements


### Bug Fixes
### Bug Fixes

* Fail fast rather than emitting `PANIC`s when attempting to generate code for ill-formed programs.
18 changes: 16 additions & 2 deletions pkg/codegen/java/gen_program.go
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,20 @@ func (g *generator) genResource(w io.Writer, resource *pcl.Resource) {
}
g.genTrivia(w, resource.Definition.Tokens.GetOpenBrace())

if resource.Schema != nil {
for _, input := range resource.Inputs {
// We traverse the set of resource input types to make sure that this attribute appears in the schema.
// However, we'll use the type we've already computed rather than the result of the traversal since the
// latter will typically be a union of the type we've computed and one or more output types. This may result
// in inaccurate code generation later on. Arguably this is a bug in the generator, but this will have to do
// for now.
_, diagnostics := resource.InputType.Traverse(hcl.TraverseAttr{Name: input.Name})
g.diagnostics = append(g.diagnostics, diagnostics...)
value := g.lowerExpression(input.Value, input.Type())
input.Value = value
}
}

instantiate := func(resName string) {
resourceProperties := typedResourceProperties(resource)
if len(resource.Inputs) == 0 && !hasCustomResourceOptions(resource) {
Expand All @@ -916,13 +930,13 @@ func (g *generator) genResource(w io.Writer, resource *pcl.Resource) {
g.Fgen(w, ")")
} else {
g.Fgenf(w, "new %s(%s, %s.builder()", resourceTypeName, resName, resourceArgs)
g.Fgenf(w, "%s\n", g.Indent)
g.Fgen(w, "\n")
g.Indented(func() {
for _, attr := range resource.Inputs {
attributeIdent := names.MakeValidIdentifier(attr.Name)
attributeSchemaType := resourceProperties[attr.Name]
g.currentResourcePropertyType = attributeSchemaType
g.Fgenf(w, "%s.%s(%.v)\n", g.Indent, attributeIdent, g.lowerExpression(attr.Value, attr.Type()))
g.Fgenf(w, "%s.%s(%.v)\n", g.Indent, attributeIdent, attr.Value)
}

if !hasCustomResourceOptions(resource) {
Expand Down
30 changes: 15 additions & 15 deletions pkg/codegen/testing/test/testdata/aws-eks-pp/java/aws-eks.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,20 @@ public static void main(String[] args) {

public static void stack(Context ctx) {
// VPC
var eksVpc = new Vpc("eksVpc", VpcArgs.builder()
var eksVpc = new Vpc("eksVpc", VpcArgs.builder()
.cidrBlock("10.100.0.0/16")
.instanceTenancy("default")
.enableDnsHostnames(true)
.enableDnsSupport(true)
.tags(Map.of("Name", "pulumi-eks-vpc"))
.build());

var eksIgw = new InternetGateway("eksIgw", InternetGatewayArgs.builder()
var eksIgw = new InternetGateway("eksIgw", InternetGatewayArgs.builder()
.vpcId(eksVpc.id())
.tags(Map.of("Name", "pulumi-vpc-ig"))
.build());

var eksRouteTable = new RouteTable("eksRouteTable", RouteTableArgs.builder()
var eksRouteTable = new RouteTable("eksRouteTable", RouteTableArgs.builder()
.vpcId(eksVpc.id())
.routes(RouteTableRouteArgs.builder()
.cidrBlock("0.0.0.0/0")
Expand All @@ -73,7 +73,7 @@ public static void stack(Context ctx) {
final var vpcSubnet = zones.applyValue(getAvailabilityZonesResult -> {
final var resources = new ArrayList<Subnet>();
for (var range : KeyedValue.of(getAvailabilityZonesResult.names()) {
var resource = new Subnet("vpcSubnet-" + range.key(), SubnetArgs.builder()
var resource = new Subnet("vpcSubnet-" + range.key(), SubnetArgs.builder()
.assignIpv6AddressOnCreation(false)
.vpcId(eksVpc.id())
.mapPublicIpOnLaunch(true)
Expand All @@ -91,7 +91,7 @@ public static void stack(Context ctx) {
final var rta = zones.applyValue(getAvailabilityZonesResult -> {
final var resources = new ArrayList<RouteTableAssociation>();
for (var range : KeyedValue.of(getAvailabilityZonesResult.names()) {
var resource = new RouteTableAssociation("rta-" + range.key(), RouteTableAssociationArgs.builder()
var resource = new RouteTableAssociation("rta-" + range.key(), RouteTableAssociationArgs.builder()
.routeTableId(eksRouteTable.id())
.subnetId(vpcSubnet[range.key()].id())
.build());
Expand All @@ -104,7 +104,7 @@ public static void stack(Context ctx) {

final var subnetIds = vpcSubnet.stream().map(element -> element.id()).collect(toList());

var eksSecurityGroup = new SecurityGroup("eksSecurityGroup", SecurityGroupArgs.builder()
var eksSecurityGroup = new SecurityGroup("eksSecurityGroup", SecurityGroupArgs.builder()
.vpcId(eksVpc.id())
.description("Allow all HTTP(s) traffic to EKS Cluster")
.tags(Map.of("Name", "pulumi-cluster-sg"))
Expand All @@ -126,7 +126,7 @@ public static void stack(Context ctx) {
.build());

// EKS Cluster Role
var eksRole = new Role("eksRole", RoleArgs.builder()
var eksRole = new Role("eksRole", RoleArgs.builder()
.assumeRolePolicy(serializeJson(
jsonObject(
jsonProperty("Version", "2012-10-17"),
Expand All @@ -141,18 +141,18 @@ public static void stack(Context ctx) {
)))
.build());

var servicePolicyAttachment = new RolePolicyAttachment("servicePolicyAttachment", RolePolicyAttachmentArgs.builder()
var servicePolicyAttachment = new RolePolicyAttachment("servicePolicyAttachment", RolePolicyAttachmentArgs.builder()
.role(eksRole.id())
.policyArn("arn:aws:iam::aws:policy/AmazonEKSServicePolicy")
.build());

var clusterPolicyAttachment = new RolePolicyAttachment("clusterPolicyAttachment", RolePolicyAttachmentArgs.builder()
var clusterPolicyAttachment = new RolePolicyAttachment("clusterPolicyAttachment", RolePolicyAttachmentArgs.builder()
.role(eksRole.id())
.policyArn("arn:aws:iam::aws:policy/AmazonEKSClusterPolicy")
.build());

// EC2 NodeGroup Role
var ec2Role = new Role("ec2Role", RoleArgs.builder()
var ec2Role = new Role("ec2Role", RoleArgs.builder()
.assumeRolePolicy(serializeJson(
jsonObject(
jsonProperty("Version", "2012-10-17"),
Expand All @@ -167,23 +167,23 @@ public static void stack(Context ctx) {
)))
.build());

var workerNodePolicyAttachment = new RolePolicyAttachment("workerNodePolicyAttachment", RolePolicyAttachmentArgs.builder()
var workerNodePolicyAttachment = new RolePolicyAttachment("workerNodePolicyAttachment", RolePolicyAttachmentArgs.builder()
.role(ec2Role.id())
.policyArn("arn:aws:iam::aws:policy/AmazonEKSWorkerNodePolicy")
.build());

var cniPolicyAttachment = new RolePolicyAttachment("cniPolicyAttachment", RolePolicyAttachmentArgs.builder()
var cniPolicyAttachment = new RolePolicyAttachment("cniPolicyAttachment", RolePolicyAttachmentArgs.builder()
.role(ec2Role.id())
.policyArn("arn:aws:iam::aws:policy/AmazonEKSCNIPolicy")
.build());

var registryPolicyAttachment = new RolePolicyAttachment("registryPolicyAttachment", RolePolicyAttachmentArgs.builder()
var registryPolicyAttachment = new RolePolicyAttachment("registryPolicyAttachment", RolePolicyAttachmentArgs.builder()
.role(ec2Role.id())
.policyArn("arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryReadOnly")
.build());

// EKS Cluster
var eksCluster = new Cluster("eksCluster", ClusterArgs.builder()
var eksCluster = new Cluster("eksCluster", ClusterArgs.builder()
.roleArn(eksRole.arn())
.tags(Map.of("Name", "pulumi-eks-cluster"))
.vpcConfig(ClusterVpcConfigArgs.builder()
Expand All @@ -193,7 +193,7 @@ public static void stack(Context ctx) {
.build())
.build());

var nodeGroup = new NodeGroup("nodeGroup", NodeGroupArgs.builder()
var nodeGroup = new NodeGroup("nodeGroup", NodeGroupArgs.builder()
.clusterName(eksCluster.name())
.nodeGroupName("pulumi-eks-nodegroup")
.nodeRoleArn(ec2Role.arn())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public static void stack(Context ctx) {
.build());

// Create a security group that permits HTTP ingress and unrestricted egress.
var webSecurityGroup = new SecurityGroup("webSecurityGroup", SecurityGroupArgs.builder()
var webSecurityGroup = new SecurityGroup("webSecurityGroup", SecurityGroupArgs.builder()
.vpcId(vpc.applyValue(getVpcResult -> getVpcResult.id()))
.egress(SecurityGroupEgressArgs.builder()
.protocol("-1")
Expand All @@ -73,7 +73,7 @@ public static void stack(Context ctx) {
var cluster = new Cluster("cluster");

// Create an IAM role that can be used by our service's task.
var taskExecRole = new Role("taskExecRole", RoleArgs.builder()
var taskExecRole = new Role("taskExecRole", RoleArgs.builder()
.assumeRolePolicy(serializeJson(
jsonObject(
jsonProperty("Version", "2008-10-17"),
Expand All @@ -88,25 +88,25 @@ public static void stack(Context ctx) {
)))
.build());

var taskExecRolePolicyAttachment = new RolePolicyAttachment("taskExecRolePolicyAttachment", RolePolicyAttachmentArgs.builder()
var taskExecRolePolicyAttachment = new RolePolicyAttachment("taskExecRolePolicyAttachment", RolePolicyAttachmentArgs.builder()
.role(taskExecRole.name())
.policyArn("arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy")
.build());

// Create a load balancer to listen for HTTP traffic on port 80.
var webLoadBalancer = new LoadBalancer("webLoadBalancer", LoadBalancerArgs.builder()
var webLoadBalancer = new LoadBalancer("webLoadBalancer", LoadBalancerArgs.builder()
.subnets(subnets.applyValue(getSubnetIdsResult -> getSubnetIdsResult.ids()))
.securityGroups(webSecurityGroup.id())
.build());

var webTargetGroup = new TargetGroup("webTargetGroup", TargetGroupArgs.builder()
var webTargetGroup = new TargetGroup("webTargetGroup", TargetGroupArgs.builder()
.port(80)
.protocol("HTTP")
.targetType("ip")
.vpcId(vpc.applyValue(getVpcResult -> getVpcResult.id()))
.build());

var webListener = new Listener("webListener", ListenerArgs.builder()
var webListener = new Listener("webListener", ListenerArgs.builder()
.loadBalancerArn(webLoadBalancer.arn())
.port(80)
.defaultActions(ListenerDefaultActionArgs.builder()
Expand All @@ -116,7 +116,7 @@ public static void stack(Context ctx) {
.build());

// Spin up a load balanced service running NGINX
var appTask = new TaskDefinition("appTask", TaskDefinitionArgs.builder()
var appTask = new TaskDefinition("appTask", TaskDefinitionArgs.builder()
.family("fargate-task-definition")
.cpu("256")
.memory("512")
Expand All @@ -135,7 +135,7 @@ public static void stack(Context ctx) {
))))
.build());

var appService = new Service("appService", ServiceArgs.builder()
var appService = new Service("appService", ServiceArgs.builder()
.cluster(cluster.arn())
.desiredCount(5)
.launchType("FARGATE")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public static void main(String[] args) {

public static void stack(Context ctx) {
// Create a bucket and expose a website index document
var siteBucket = new Bucket("siteBucket", BucketArgs.builder()
var siteBucket = new Bucket("siteBucket", BucketArgs.builder()
.website(BucketWebsiteArgs.builder()
.indexDocument("index.html")
.build())
Expand All @@ -38,7 +38,7 @@ public static void stack(Context ctx) {

// For each file in the directory, create an S3 object stored in `siteBucket`
for (var range : readDir(siteDir)) {
new BucketObject("files-" + range.key(), BucketObjectArgs.builder()
new BucketObject("files-" + range.key(), BucketObjectArgs.builder()
.bucket(siteBucket.id())
.key(range.value())
.source(new FileAsset(Paths.get(siteDir, range.value())))
Expand All @@ -48,7 +48,7 @@ public static void stack(Context ctx) {

// set the MIME type of the file
// Set the access policy for the bucket so all objects are readable
var bucketPolicy = new BucketPolicy("bucketPolicy", BucketPolicyArgs.builder()
var bucketPolicy = new BucketPolicy("bucketPolicy", BucketPolicyArgs.builder()
.bucket(siteBucket.id())
.policy(siteBucket.id().applyValue(id -> serializeJson(
jsonObject(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ public static void main(String[] args) {
public static void stack(Context ctx) {
var logs = new Bucket("logs");

var bucket = new Bucket("bucket", BucketArgs.builder()
var bucket = new Bucket("bucket", BucketArgs.builder()
.loggings(BucketLoggingArgs.builder()
.targetBucket(logs.bucket())
.build())
.build());

var indexFile = new BucketObject("indexFile", BucketObjectArgs.builder()
var indexFile = new BucketObject("indexFile", BucketObjectArgs.builder()
.bucket(bucket.id())
.source(Files.readString(Paths.get("./index.html")))
.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public static void main(String[] args) {

public static void stack(Context ctx) {
// Create a new security group for port 80.
var securityGroup = new SecurityGroup("securityGroup", SecurityGroupArgs.builder()
var securityGroup = new SecurityGroup("securityGroup", SecurityGroupArgs.builder()
.ingress(SecurityGroupIngressArgs.builder()
.protocol("tcp")
.fromPort(0)
Expand All @@ -44,7 +44,7 @@ public static void stack(Context ctx) {
.build());

// Create a simple web server using the startup script for the instance.
var server = new Instance("server", InstanceArgs.builder()
var server = new Instance("server", InstanceArgs.builder()
.tags(Map.of("Name", "web-server-www"))
.instanceType("t2.micro")
.securityGroups(securityGroup.name())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public static void main(String[] args) {
}

public static void stack(Context ctx) {
var frontDoor = new FrontDoor("frontDoor", FrontDoorArgs.builder()
var frontDoor = new FrontDoor("frontDoor", FrontDoorArgs.builder()
.routingRules(RoutingRuleArgs.builder()
.routeConfiguration(ForwardingConfigurationArgs.builder()
.odataType("#Microsoft.Azure.FrontDoor.Models.FrontdoorForwardingConfiguration")
Expand All @@ -33,7 +33,7 @@ public static void stack(Context ctx) {
.build())
.build());

var endpoint = new Endpoint("endpoint", EndpointArgs.builder()
var endpoint = new Endpoint("endpoint", EndpointArgs.builder()
.deliveryPolicy(EndpointPropertiesUpdateParametersDeliveryPolicyArgs.builder()
.rules(DeliveryRuleArgs.builder()
.actions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static void stack(Context ctx) {
final var locationParam = config.get("locationParam").orElse(resourceGroupVar.applyValue(getResourceGroupResult -> getResourceGroupResult.location()));
final var storageAccountTierParam = config.get("storageAccountTierParam").orElse("Standard");
final var storageAccountTypeReplicationParam = config.get("storageAccountTypeReplicationParam").orElse("LRS");
var storageAccountResource = new Account("storageAccountResource", AccountArgs.builder()
var storageAccountResource = new Account("storageAccountResource", AccountArgs.builder()
.name(storageAccountNameParam)
.accountKind("StorageV2")
.location(locationParam)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static void main(String[] args) {
}

public static void stack(Context ctx) {
var pulumi_kubernetes_operatorDeployment = new Deployment("pulumi_kubernetes_operatorDeployment", DeploymentArgs.builder()
var pulumi_kubernetes_operatorDeployment = new Deployment("pulumi_kubernetes_operatorDeployment", DeploymentArgs.builder()
.apiVersion("apps/v1")
.kind("Deployment")
.metadata(ObjectMetaArgs.builder()
Expand Down Expand Up @@ -85,7 +85,7 @@ public static void stack(Context ctx) {
.build())
.build());

var pulumi_kubernetes_operatorRole = new Role("pulumi_kubernetes_operatorRole", RoleArgs.builder()
var pulumi_kubernetes_operatorRole = new Role("pulumi_kubernetes_operatorRole", RoleArgs.builder()
.apiVersion("rbac.authorization.k8s.io/v1")
.kind("Role")
.metadata(ObjectMetaArgs.builder()
Expand Down Expand Up @@ -168,7 +168,7 @@ public static void stack(Context ctx) {
.build())
.build());

var pulumi_kubernetes_operatorRoleBinding = new RoleBinding("pulumi_kubernetes_operatorRoleBinding", RoleBindingArgs.builder()
var pulumi_kubernetes_operatorRoleBinding = new RoleBinding("pulumi_kubernetes_operatorRoleBinding", RoleBindingArgs.builder()
.kind("RoleBinding")
.apiVersion("rbac.authorization.k8s.io/v1")
.metadata(ObjectMetaArgs.builder()
Expand All @@ -185,7 +185,7 @@ public static void stack(Context ctx) {
.build())
.build());

var pulumi_kubernetes_operatorServiceAccount = new ServiceAccount("pulumi_kubernetes_operatorServiceAccount", ServiceAccountArgs.builder()
var pulumi_kubernetes_operatorServiceAccount = new ServiceAccount("pulumi_kubernetes_operatorServiceAccount", ServiceAccountArgs.builder()
.apiVersion("v1")
.kind("ServiceAccount")
.metadata(ObjectMetaArgs.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public static void main(String[] args) {
}

public static void stack(Context ctx) {
var bar = new Pod("bar", PodArgs.builder()
var bar = new Pod("bar", PodArgs.builder()
.apiVersion("v1")
.kind("Pod")
.metadata(ObjectMetaArgs.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ public static void main(String[] args) {
}

public static void stack(Context ctx) {
var provider = new Provider("provider", ProviderArgs.builder()
.enableServerSideApply(true)
var provider = new Provider("provider", ProviderArgs.builder()
.enableDryRun(true)
.build());

var argocd_serverDeployment = new Deployment("argocd_serverDeployment", DeploymentArgs.builder()
var argocd_serverDeployment = new Deployment("argocd_serverDeployment", DeploymentArgs.builder()
.apiVersion("apps/v1")
.kind("Deployment")
.metadata(ObjectMetaArgs.builder()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
resource provider "pulumi:providers:kubernetes" {
enableServerSideApply = true
enableDryRun = true
}

resource argocd_serverDeployment "kubernetes:apps/v1:Deployment" {
Expand Down

0 comments on commit b69848a

Please sign in to comment.