Skip to content
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.

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

PANIC generated into public-facing documentation #1341

Closed
t0yv0 opened this issue Apr 18, 2024 · 9 comments
Closed

PANIC generated into public-facing documentation #1341

t0yv0 opened this issue Apr 18, 2024 · 9 comments
Assignees
Labels
area/codegen impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Milestone

Comments

@t0yv0
Copy link
Member

t0yv0 commented Apr 18, 2024

What happened?

Currently as spotted by @VenelinMartinov RandomInteger docs generate a PANIC into Java code, where other languages work fine. This is using pulumi convert to perform the translation.

Example

# The following example shows how to generate a random priority
# between 1 and 50000 for a aws_alb_listener_rule resource:

resource "random_integer" "priority" {
  min = 1
  max = 50000
  keepers = {
    # Generate a new integer each time we switch to a new listener ARN
    listener_arn = var.listener_arn
  }
}

resource "aws_alb_listener_rule" "main" {
  listener_arn = random_integer.priority.keepers.listener_arn
  priority     = random_integer.priority.result

  action {
    type             = "forward"
    target_group_arn = var.target_group_arn
  }
  # ... (other aws_alb_listener_rule arguments) ...
}

Getting this rendered:

package generated_program;

import com.pulumi.Context;
import com.pulumi.Pulumi;
import com.pulumi.core.Output;
import com.pulumi.random.RandomInteger;
import com.pulumi.random.RandomIntegerArgs;
import com.pulumi.aws.albListenerRule;
import com.pulumi.aws.AlbListenerRuleArgs;
import java.util.List;
import java.util.ArrayList;
import java.util.Map;
import java.io.File;
import java.nio.file.Files;
import java.nio.file.Paths;

public class App {
    public static void main(String[] args) {
        Pulumi.run(App::stack);
    }

    public static void stack(Context ctx) {
        // The following example shows how to generate a random priority
        // between 1 and 50000 for a aws_alb_listener_rule resource:
        var priority = new RandomInteger("priority", RandomIntegerArgs.builder()        
            .min(1)
            .max(50000)
            .keepers(Map.of("listener_arn", listenerArn))
            .build());

        var main = new AlbListenerRule("main", AlbListenerRuleArgs.builder()        
            .listenerArn(priority.keepers().applyValue(keepers -> keepers.listenerArn()))
            .priority(priority.result())
            .action(%!v(PANIC=Format method: runtime error: invalid memory address or nil pointer dereference))
            .build());

    }
}

Python translation:

import pulumi
import pulumi_aws as aws
import pulumi_random as random

# The following example shows how to generate a random priority
# between 1 and 50000 for a aws_alb_listener_rule resource:
priority = random.RandomInteger("priority",
    min=1,
    max=50000,
    keepers={
        "listener_arn": listener_arn,
    })
main = aws.index.AlbListenerRule("main",
    listener_arn=priority.keepers.listener_arn,
    priority=priority.result,
    action=[{
        type: forward,
        targetGroupArn: target_group_arn,
    }])

Output of pulumi about

N/A

Additional context

N/A

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).

@t0yv0 t0yv0 added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Apr 18, 2024
@t0yv0
Copy link
Member Author

t0yv0 commented Apr 18, 2024

Since the page https://www.pulumi.com/registry/packages/random/api-docs/randominteger/ does not have this problem yet, this must be a new issue generated by the converter rollout.

@justinvp justinvp added area/codegen impact/panic This bug represents a panic or unexpected crash p1 A bug severe enough to be the next item assigned to an engineer and removed needs-triage Needs attention from the triage team labels Apr 19, 2024
@justinvp justinvp added this to the 0.103 milestone Apr 19, 2024
@Zaid-Ajaj
Copy link
Contributor

@t0yv0 I've been investigating this issue. I am using the following:

  • Pulumi CLI v3.114.0
  • pulumi-converter-terraform v1.0.16

Running pulumi convert --from terraform --language python --out py against the example above, gives me different results:

import pulumi
import pulumi_aws as aws
import pulumi_random as random

priority = random.RandomInteger("priority",
    min=1,
    max=50000,
    keepers={
        "listener_arn": listener_arn,
    })
main = aws.alb.ListenerRule("main",
    listener_arn=priority.keepers["listenerArn"],
    priority=priority.result,
    actions=[aws.alb.ListenerRuleActionArgs(
        type="forward",
        target_group_arn=target_group_arn,
    )])

Notice the subtle difference in actions where the only element in the array is a typed object aws.alb.ListenerRuleActionArgs(...) not just {...}. This to me suggests that the PCL binder couldn't load the schema of aws while generating examples docs for random 🤔 That would be the case if you are disabling automatic plugin acquisition during this phase (i.e. only schema of random being available)

The specific java issue is that when it tries load the schema type of an object and it can't find it, it panics. That means program-gen here needs to be more resilient with these cases.

That said, when we do have access to the schema of aws, program generation for java works fine. Running:

pulumi convert --from terraform --language java --out java

Gives:

package generated_program;

// imports ... 

public class App {
    public static void main(String[] args) {
        Pulumi.run(App::stack);
    }

    public static void stack(Context ctx) {
        var priority = new RandomInteger("priority", RandomIntegerArgs.builder()        
            .min(1)
            .max(50000)
            .keepers(Map.of("listener_arn", listenerArn))
            .build());

        var main = new ListenerRule("main", ListenerRuleArgs.builder()        
            .listenerArn(priority.keepers().applyValue(keepers -> keepers.listenerArn()))
            .priority(priority.result())
            .actions(ListenerRuleActionArgs.builder()
                .type("forward")
                .targetGroupArn(targetGroupArn)
                .build())
            .build());

    }
}

@t0yv0
Copy link
Member Author

t0yv0 commented May 1, 2024

That's a brilliant suggestion, let me go ahead and do a pass at fixing this in pulumi-random.

that would be the case if you are disabling automatic plugin acquisition during this phase

We do try to run with disabled plugin acquisition indeed as much as possible.

QQ: is there a chance we can make a switch to fail hard and fast when a schema for something is not available, or perhaps not listed in a config file ? In this provider build scenario, we would very much rather have that, so that'd guide us quickly in the direction of pinning and making available all the required dependencies.

@t0yv0
Copy link
Member Author

t0yv0 commented May 1, 2024

Similar issue in pulumi/pulumi-aws#3125 - still a problem, I reopened it; checking up on it as well.

@t0yv0
Copy link
Member Author

t0yv0 commented May 1, 2024

Random provider is unblocked, good news. Still struggling with completely closing out the issues in AWS, I opened a few more tickets with concrete examples.

@t0yv0
Copy link
Member Author

t0yv0 commented May 1, 2024

You can close this one as no-repro (once the AWS dependency in random is updated to 6.x).

@justinvp justinvp modified the milestones: 0.103, 0.104 May 3, 2024
@justinvp justinvp assigned lunaris and unassigned Zaid-Ajaj May 8, 2024
@justinvp
Copy link
Member

justinvp commented May 8, 2024

@lunaris, does #1357 address this issue as well?

@t0yv0
Copy link
Member Author

t0yv0 commented May 8, 2024

FWIW since we updated the dependencies in Random it no longer reproduces.

@justinvp justinvp added the resolution/fixed This issue was fixed label May 8, 2024
@justinvp
Copy link
Member

justinvp commented May 8, 2024

I'm going to go ahead and close this as fixed by #1357 (and no longer repos anyway given updating the deps in Random).

@justinvp justinvp closed this as completed May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/codegen impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec p1 A bug severe enough to be the next item assigned to an engineer resolution/fixed This issue was fixed
Projects
None yet
4 participants