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

Adding teams breaks RepositoryEnvironment reviewers and causes a panic #278

Closed
JDTX0 opened this issue Dec 22, 2022 · 6 comments · Fixed by pulumi/pulumi-terraform-bridge#1243
Assignees
Labels
awaiting-feedback Blocked on input from the author impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed

Comments

@JDTX0
Copy link
Contributor

JDTX0 commented Dec 22, 2022

What happened?

Adding additional teams using github.Team causes existing RepositoryEnvironments resources fail to fetch team id values for the reviewers and then causes an interface conversion panic on preview & up.

Even if the new team isn't related in any way to the existing RepositoryEnvironment, it still panics. In other words, the new team being added is not added to the reviewers, so it should be unaffected, but it still is.

Here's the panic:

   panic: interface conversion: interface {} is string, not int
    goroutine 6093 [running]:
    github.com/hashicorp/terraform-plugin-sdk/helper/schema.SerializeValueForHash(0xc0017ed960?, {0x20bfec0?, 0x27aa360?}, 0xc000f82c70?)
    	/home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/helper/schema/serialize.go:24 +0x2da
    github.com/hashicorp/terraform-plugin-sdk/helper/schema.HashSchema.func1({0x20bfec0?, 0x27aa360?})
    	/home/runner/go/pkg/mod/github.com/hashicorp/[email protected]/helper/schema/set.go:43 +0x4b
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfshim/sdk-v1.v1Schema.SetHash({0x0?}, {0x20bfec0, 0x27aa360})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/tfshim/sdk-v1/schema.go:180 +0x43
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.visitPropertyValue({0xc00181af30, 0x11}, {0xc00181af18, 0x12}, {{0x2099d00?, 0xc001769fb0?}}, {0x27c9700?, 0xc000bb52c0}, 0x0, 0x0, ...)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/tfbridge/diff.go:114 +0x812
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.visitPropertyValue({0xc0019a6230, 0xb}, {0xc0019a6220, 0xc}, {{0x22ebf40?, 0xc00089db30?}}, {0x27c9640?, 0xc00184a840}, 0x0, 0x0, ...)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/tfbridge/diff.go:146 +0x425
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.visitPropertyValue({0xc0019a6210, 0x9}, {0xc0018523a0, 0x9}, {{0x2099d00?, 0xc001996018?}}, {0x27c9700?, 0xc000bb5680}, 0x0, 0x0, ...)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/tfbridge/diff.go:122 +0x8fa
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.doIgnoreChanges({0x27c1778, 0xc000b75a70}, 0xc00198d220?, 0x27b34c0?, 0xc000b2c240?, {0x0, 0x0, 0xc000f83560?}, {0x27c3520, 0xc0017ecc40})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/tfbridge/diff.go:260 +0x3ce
    github.com/pulumi/pulumi-terraform-bridge/v3/pkg/tfbridge.(*Provider).Diff(0xc000b81680, {0x27bc308?, 0xc00089c9f0?}, 0xc00178bf10)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi-terraform-bridge/[email protected]/pkg/tfbridge/provider.go:778 +0x608
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Diff_Handler.func1({0x27bc308, 0xc00089c9f0}, {0x22bc2e0?, 0xc00178bf10})
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/[email protected]/proto/go/provider.pb.go:3728 +0x78
    github.com/grpc-ecosystem/grpc-opentracing/go/otgrpc.OpenTracingServerInterceptor.func1({0x27bc308, 0xc00167fec0}, {0x22bc2e0, 0xc00178bf10}, 0xc0017ec740, 0xc001769c80)
    	/home/runner/go/pkg/mod/github.com/grpc-ecosystem/[email protected]/go/otgrpc/server.go:57 +0x3f9
    github.com/pulumi/pulumi/sdk/v3/proto/go._ResourceProvider_Diff_Handler({0x233f100?, 0xc000b81680}, {0x27bc308, 0xc00167fec0}, 0xc00178bea0, 0xc000b79040)
    	/home/runner/go/pkg/mod/github.com/pulumi/pulumi/sdk/[email protected]/proto/go/provider.pb.go:3730 +0x138
    google.golang.org/grpc.(*Server).processUnaryRPC(0xc000bfc000, {0x27c39a0, 0xc00049f860}, 0xc001570d80, 0xc00089c0c0, 0x32eb048, 0x0)
    	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:1340 +0xd23
    google.golang.org/grpc.(*Server).handleStream(0xc000bfc000, {0x27c39a0, 0xc00049f860}, 0xc001570d80, 0x0)
    	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:1713 +0xa2f
    google.golang.org/grpc.(*Server).serveStreams.func1.2()
    	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:965 +0x98
    created by google.golang.org/grpc.(*Server).serveStreams.func1
    	/home/runner/go/pkg/mod/google.golang.org/[email protected]/server.go:963 +0x28a

I ran pulumi preview --logtostderr --logflow -v=9 2>&1 | tee -a debug.log and got some debug info:

    I1222 15:04:30.804119   13631 schema.go:475] Created Terraform input: environment = redacted
    I1222 15:04:30.804128   13631 schema.go:475] Created Terraform input: repository = redacted
    I1222 15:04:30.804137   13631 schema.go:475] Created Terraform input: teams = [74D93920-ED26-11E3-AC10-0800200C9A66]
    I1222 15:04:30.804147   13631 schema.go:485] Terraform input teams = []interface {}{"74D93920-ED26-11E3-AC10-0800200C9A66"}
    I1222 15:04:30.804157   13631 schema.go:475] Created Terraform input: reviewers = [map[teams:[74D93920-ED26-11E3-AC10-0800200C9A66]]]
    I1222 15:04:30.804166   13631 schema.go:475] Created Terraform input: wait_timer = 1
    I1222 15:04:30.804177   13631 schema.go:475] Created Terraform input: custom_branch_policies = true
    I1222 15:04:30.804182   13631 schema.go:475] Created Terraform input: protected_branches = false
    I1222 15:04:30.804188   13631 schema.go:485] Terraform input custom_branch_policies = true
    I1222 15:04:30.804192   13631 schema.go:485] Terraform input protected_branches = false
    I1222 15:04:30.804205   13631 schema.go:475] Created Terraform input: deployment_branch_policy = [map[custom_branch_policies:true protected_branches:false]]
    I1222 15:04:30.804213   13631 schema.go:485] Terraform input environment = "redacted"
    I1222 15:04:30.804217   13631 schema.go:485] Terraform input repository = "redacted"
    I1222 15:04:30.804222   13631 schema.go:485] Terraform input reviewers = []interface {}{map[string]interface {}{"teams":[]interface {}{"74D93920-ED26-11E3-AC10-0800200C9A66"}}}
    I1222 15:04:30.804230   13631 schema.go:485] Terraform input wait_timer = 1
    I1222 15:04:30.804232   13631 schema.go:485] Terraform input deployment_branch_policy = []interface {}{map[string]interface {}{"custom_branch_policies":true, "protected_branches":false}}

Most interesting to me is that the reviewers is getting set to a map of teams containing 74D93920-ED26-11E3-AC10-0800200C9A66 which is a special value in Terraform that represents an unknown variable

I imagine that value (74D93920-ED26-11E3-AC10-0800200C9A66) is what's causing the panic as it's a string, and the interface expects an integer.

Oddly this doesn't happen for every RepositoryEnvironment. There's a few RepositoryEnvironments above that one in the log that get the correct team ids and don't have that 74D93920-ED26-11E3-AC10-0800200C9A66 string anywhere.

I've currently been working around this bug by manually creating the team on the GitHub side, then importing the team into Pulumi's state using pulumi import, then the preview/up works fine with the new team defined.

As an aside, it'd be great if we could pass the custom team names directly instead of having to provide the ID's and resolve those id's ourselves. I see something similar was added recently for branch protection rules in integrations/terraform-provider-github#1020 -- If that's out of the scope of this (which I assume it is, since this is just a bridge of sorts), then I'm happy to open a feature request upstream.

Steps to reproduce

  1. Define one or more teams using github.Team, fully apply the changes and ensure the teams are created in GH
  2. Define one or more RepositoryEnvironments with some team ids as the reviewers, fully apply the changes, ensure the repo environments are created
  3. Define a new team in the code, attempt to run pulumi preview

Here's the code for function we're using to map team names to team ids:

function getTeamIds(
  slugs: string[],
  teams: github.Team[]
): pulumi.Output<number[]> {
  return pulumi
    .all(
      teams.map(t =>
        pulumi
          .all([t.slug, t.id])
          .apply(([slug, id]) => (slugs.includes(slug) ? parseInt(id) : null))
      )
    )
    .apply(ids => ids.filter(v => v !== null) as number[]);
}

And here's the code that's calling that function:

      repo.environments?.forEach((v, envK) => {
        // convert reviewer team names to team IDs
        var envTeams: pulumi.Output<number[]> | undefined = v.reviewers?.teams
          ? getTeamIds(v.reviewers?.teams, teams)
          : undefined;

        var env = new github.RepositoryEnvironment(
          `${slug}-${envK}`,
          {
            environment: envK,
            repository: r.name,
            deploymentBranchPolicy: policy,
            reviewers: [
              {
                teams: envTeams,
              },
            ],
            waitTimer: v.waitTimer,
          },
          { provider: p }
        );

Expected Behavior

The preview/up works as expected, and the addition of a new team does not cause a panic

Actual Behavior

The preview panics with the interface conversion error.

Output of pulumi about

Note: I've stripped the resources and some info here, but the relevant software versions are included. If you really need the full output, please reach out to me privately.

CLI
Version      3.50.2
Go Version   go1.19.4
Go Compiler  gc

Plugins
NAME    VERSION
gcp     5.11.0
github  5.1.0
nodejs  unknown

Host
OS       darwin
Version  12.6
Arch     x86_64

This project is written in nodejs: executable='/Users/jonathantooker/.nvm/versions/node/v14.17.6/bin/node' version='v14.17.6'

Current Stack: [redacted]]

TYPE                                                                        URN
[redacted]


Found no pending operations associated with [redacted]

Backend
Name           pulumi.com
[redacted]

Dependencies:
NAME                       VERSION
@pulumi/gcp                5.11.0
@pulumi/github             5.1.0
@pulumi/pulumi             3.50.2
@types/node                16.0.0
axios                      0.21.4
markdownlint-rule-helpers  0.16.0
yaml                       2.1.1

Pulumi locates its logs in /var/folders/sw/q1tjy96d5tn_h3xfdc5968140000gp/T/ by default

Also note: I've tried upgrading everything to the latest versions available, it made no difference, still panics.

Additional context

No response

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).

@JDTX0 JDTX0 added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Dec 22, 2022
@mikhailshilkov
Copy link
Member

Hi @JDTX0

I'm trying to reproduce your issue... what is repo.environments in your example? Is it needed for a repro? Do you happen to have a standalone program that I could run to see the same error? Thank you!

@mikhailshilkov mikhailshilkov added impact/panic This bug represents a panic or unexpected crash awaiting-feedback Blocked on input from the author and removed needs-triage Needs attention from the triage team labels Dec 26, 2022
@JDTX0
Copy link
Contributor Author

JDTX0 commented Dec 27, 2022

@mikhailshilkov Thanks for your fast reply. It's a map containing values to pass to RepositoryEnvironment

Here's an example of the map we're passing:

  environments: new Map([
    [
      'test',
      {
        deploymentBranchPolicy: {
          customBranchPolicies: ['test', 'release/*'],
          protectedBranches: false,
        },
        reviewers: {
          teams: ['team-name-1', 'team-name-2', 'team-name-3'],
        },
        waitTimer: 1,
      },
    ],

Note: values have been replaced with sample values, but they contain real team names in our env

Most importantly, we're passing the team names in the reviewers object. The team names are then converted to ID's in the getTeamIds function prior to passing to RepositoryEnvironment

Just for some context, we use maps for pretty much everything, and we use foreach/loops around map entries to call the appropriate functions. Instead of repeating code over and over, we just iterate over map entries defined in TS and create resources that way.

Unfortunately I don't have a standalone program for reproducing this. I can probably try to create one if you're unable to reproduce the problem, but all of the code to reproduce should be available in this report.

@mikhailshilkov
Copy link
Member

What does this step mean exactly?

  1. Define a new team in the code, attempt to run pulumi preview

Are you adding a team to reviewers? Are you creating a new Team resource and adding it to the teams variable that isn't defined anywhere?

I can probably try to create one if you're unable to reproduce the problem

That would be quite helpful.

@JDTX0
Copy link
Contributor Author

JDTX0 commented Dec 28, 2022

@mikhailshilkov I spent some time whittling down our production code to the smallest usable example that reproduces the problem.

I've included the minimal program below. Here's how to reproduce:

Note: I created a brand new org in GitHub for testing and used a GitHub Enterprise trial for it.

  1. Create a GitHub access token (classic) here that has all perms for your testing org, and run export GITHUB_TOKEN=YOUR_TOKEN in your terminal.
  2. Edit the orgName value in index.ts (attached to this comment) to your own org name
  3. Run npm install and then pulumi up -- you should see the something like this:
     Type                                   Name                         Status
 +   pulumi:pulumi:Stack                    gh-provider-issue-repro-dev  created (0.57s)
 +   ├─ pulumi:providers:github             jdt-test-org                 created (0.32s)
 +   ├─ github:index:Team                   jdt-test-org-testteam1       created (10s)
 +   ├─ github:index:Repository             jdt-test-org-testrepo        created (10s)
 +   └─ github:index:RepositoryEnvironment  jdt-test-org-testrepo        created (1s)

Resources:
    + 5 created
  1. Edit index.ts and comment out const teams = [...[testTeam]]; (Line 14) then uncomment const teams = [...[testTeam, testTeam2]]; (Line 15) -- This adds a new team
  2. Run pulumi preview and you should get a panic.

Here's the files for the project.

index.ts

import * as pulumi from "@pulumi/pulumi";
import * as github from "@pulumi/github";
const orgName = "your-org-name";
const repoName = "testrepo";

const testTeam = {
  name: "testteam1",
};

const testTeam2 = {
  name: "testteam2",
};

const teams = [...[testTeam]];
// const teams = [...[testTeam, testTeam2]];

const providers = new Map<string, github.Provider>();
if (!providers.has(orgName)) {
  providers.set(
    orgName,
    new github.Provider(orgName, {
      owner: orgName,
    })
  );
}
const p = providers.get(orgName);

const teamsMap = teams.map((teamSettings) => {
  const team = new github.Team(
    `${orgName}-${teamSettings.name}`,
    {
      ...teamSettings,
      ...{ privacy: "closed" },
    },
    { provider: p }
  );
  return team;
});

const r = new github.Repository(
  `${orgName}-${repoName}}`,
  {
    name: `${repoName}`,
    visibility: "private",
  },
  { provider: p }
);

new github.RepositoryEnvironment(
  `${orgName}-${repoName}`,
  {
    environment: "dev",
    repository: `${repoName}`,
    reviewers: [
      {
        teams: getTeamIds(["testteam1"], teamsMap),
      },
    ],
  },
  { provider: p, dependsOn: [r] }
);

function getTeamIds(
  slugs: string[],
  teams: github.Team[]
): pulumi.Output<number[]> {
  return pulumi
    .all(
      teams.map((t) =>
        pulumi
          .all([t.slug, t.id])
          .apply(([slug, id]) => (slugs.includes(slug) ? parseInt(id) : null))
      )
    )
    .apply((ids) => ids.filter((v) => v !== null) as number[]);
}

package.json

{
    "name": "gh-provider-issue-repro",
    "main": "index.ts",
    "devDependencies": {
        "@types/node": "^14"
    },
    "dependencies": {
        "@pulumi/github": "^5.1.0",
        "@pulumi/pulumi": "^3.0.0"
    }
}

tsconfig.json

{
    "compilerOptions": {
        "strict": true,
        "outDir": "bin",
        "target": "es2016",
        "module": "commonjs",
        "moduleResolution": "node",
        "sourceMap": true,
        "experimentalDecorators": true,
        "pretty": true,
        "noFallthroughCasesInSwitch": true,
        "noImplicitReturns": true,
        "forceConsistentCasingInFileNames": true
    },
    "files": [
        "index.ts"
    ]
}

Pulumi.yaml

name: gh-provider-issue-repro
runtime: nodejs
description: A Pulumi program to reproduce a panic with adding new teams

Hope that helps. If you have any questions or need anything else, please let me know.

@JDTX0
Copy link
Contributor Author

JDTX0 commented Jan 18, 2023

I re-tested my code above with the v5.2.1 release and the panic still happens.

There was an update to the terraform provider in the v5.2.0 release that I was hoping might fix it, but no luck unfortunately.

@JDTX0
Copy link
Contributor Author

JDTX0 commented Apr 11, 2023

@mikhailshilkov Any chance someone can take a look at this issue? We're still running into the problem and we have to make teams manually + import them into the Pulumi state to work around this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-feedback Blocked on input from the author impact/panic This bug represents a panic or unexpected crash kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants