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

Create new role for sigstore-bot in root-signing-staging #395

Merged

Conversation

jku
Copy link
Member

@jku jku commented Feb 1, 2024

  • Role allows bypassing branch protections
  • This replaces the use of a separate review bot
  • Plan is to test this out in root-signing-staging, and later apply same structure to root-signing

See sigstore/github-sync#117 for more context. This is a draft until sigstore/github-sync#118 is merged.

* Role allows bypassing branch protections
* This replaces the use of a separate review bot
* Plan is to test this out in root-signing-staging, and later
  apply same structure to root-signing

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku
Copy link
Member Author

jku commented Feb 2, 2024

github-sync 118 is merged, thanks cpanato. The newest run (using github-sync adec712) still fails the same way:
Failed to load config: failed to parse github-data/sigstore/roles.yaml: failed to parse yaml: error unmarshaling JSON: while decoding JSON: json: unknown field "repositoryRoles"

So there has to be some problem in the code or this PR still. I will look into it.

@cpanato
Copy link
Member

cpanato commented Feb 2, 2024

yes, i will try to look on that as well

@jku
Copy link
Member Author

jku commented Feb 2, 2024

The parser is expecting the top level field to be "customRoles": I thought it would be "repositoryRoles" based on Config.go:

type Config struct {
	Users        []User       `yaml:"users"`
	Teams        []Team       `yaml:"teams"`
	Repositories []Repository `yaml:"repositories"`
	CustomRoles  []CustomRole `yaml:"repositoryRoles"`
}

Don't ask why I used two different names -- that was not really intentional just a result of GitHub docs using two names. I still thought the current setup would work...

@jku
Copy link
Member Author

jku commented Feb 2, 2024

Oh I am missing something in the parser code as well: func (p *Parser) Parse has custom code for the toplevel fields... Looks like I'll have to do some changes in github-sync still

Copy link

github-actions bot commented Feb 2, 2024

🍹 preview on sigstore-github-sync/sigstore/github-prod

Pulumi report
Previewing update (sigstore/github-prod)

View Live: https://app.pulumi.com/sigstore/sigstore-github-sync/github-prod/previews/a33ba646-2179-4503-b850-dbf7b11f2770

@ Previewing update.....
pulumi:pulumi:Stack: (same)
[urn=urn:pulumi:github-prod::sigstore-github-sync::pulumi:pulumi:Stack::sigstore-github-sync-github-prod]
+ github:index/organizationCustomRole:OrganizationCustomRole: (create)
    [urn=urn:pulumi:github-prod::sigstore-github-sync::github:index/organizationCustomRole:OrganizationCustomRole::write-with-bypass]
    baseRole   : "write"
    description: "write role with an additional permission to bypass branch protection"
    name       : "write-with-bypass-d37e593"
    permissions: [
        [0]: "bypass_branch_protection"
    ]
@ Previewing update....
+-github:index/repositoryCollaborator:RepositoryCollaborator: (replace)
    [id=root-signing-staging:sigstore-bot]
    [urn=urn:pulumi:github-prod::sigstore-github-sync::github:index/repositoryCollaborator:RepositoryCollaborator::root-signing-staging-sigstore-bot]
  ~ permission: "push" => "write-with-bypass"
- github:index/repositoryCollaborator:RepositoryCollaborator: (delete)
    [id=root-signing-staging:sigstore-review-bot]
    [urn=urn:pulumi:github-prod::sigstore-github-sync::github:index/repositoryCollaborator:RepositoryCollaborator::root-signing-staging-sigstore-review-bot]
    permission               : "push"
    permissionDiffSuppression: false
    repository               : "root-signing-staging"
    username                 : "sigstore-review-bot"
Resources:
+ 1 to create
- 1 to delete
+-1 to replace
3 changes. 573 unchanged

@jku
Copy link
Member Author

jku commented Feb 2, 2024

The PR in github-sync is needed before the preview should show anything as my yaml config change is currently not actually appended to pulumi config:
sigstore/github-sync#119

The name is now consistently customRoles everywhere.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
In pulumi args permission "push" usually seems to mean Github role
"write"... but that does not work for custom roles base role.

Use 'write' as the base role.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
@jku jku force-pushed the use-custom-role-in-root-signing-staging branch from b9df5ca to 83c6756 Compare February 2, 2024 11:19
@jku
Copy link
Member Author

jku commented Feb 2, 2024

Well the change seems to have happened:

     +-github:index/repositoryCollaborator:RepositoryCollaborator: (replace)
         [id=root-signing-staging:sigstore-bot]
         [urn=urn:pulumi:github-prod::sigstore-github-sync::github:index/repositoryCollaborator:RepositoryCollaborator::root-signing-staging-sigstore-bot]
       ~ permission: "push" => "write-with-bypass"

I'm happy to try this out and see what happens in root-signing-staging... Marking this ready for review. CC @haydentherapper

There's a lot of churn in the log that seems unrelated but that I don't understand:

Resources:
+ 2 to create
~ 48 to update
- 1 to delete
+-1 to replace
52 changes. 524 unchanged

is that expected?

@jku jku marked this pull request as ready for review February 2, 2024 11:30
@jku jku requested a review from a team as a code owner February 2, 2024 11:30
@haydentherapper
Copy link
Contributor

All the changes are because there’s a bug right now, where the last few changes haven’t been synced (see the failure on the last commit at HEAD)

@jku
Copy link
Member Author

jku commented Feb 7, 2024

pulumi preview now looks like expected after #397

@cpanato
Copy link
Member

cpanato commented Feb 7, 2024

re-run the job and now looks better :)

@haydentherapper
Copy link
Contributor

Can someone approve and merge now?

@bobcallaway bobcallaway merged commit 485faf7 into sigstore:main Feb 7, 2024
3 checks passed
@bobcallaway
Copy link
Member

bobcallaway commented Feb 7, 2024

202
    pulumi:pulumi:Stack: (same)
203
      [urn=urn:pulumi:github-prod::sigstore-github-sync::pulumi:pulumi:Stack::sigstore-github-sync-github-prod]
204
      + github:index/organizationCustomRole:OrganizationCustomRole: (create)
205
          [urn=urn:pulumi:github-prod::sigstore-github-sync::github:index/organizationCustomRole:OrganizationCustomRole::write-with-bypass]
206
          baseRole   : "write"
207
          description: "write role with an additional permission to bypass branch protection"
208
          name       : "write-with-bypass-3d9b256"
209
          permissions: [
210
              [0]: "bypass_branch_protection"
211
          ]
212
          --outputs:--
213
          id         : "13311"
214
  Resources:
215
      + 1 created
216
      450 unchanged
217
  Duration: 53s
218
      at Object.createCommandError (/home/runner/work/_actions/pulumi/actions/76683de37aa44910871ba6cef36557780f2e41d1/webpack:/pulumi-github-action/node_modules/@pulumi/pulumi/automation/errors.js:77:1)
219
      at Object.<anonymous> (/home/runner/work/_actions/pulumi/actions/76683de37aa44910871ba6cef36557780f2e41d1/webpack:/pulumi-github-action/node_modules/@pulumi/pulumi/automation/cmd.js:76:1)
220
      at Generator.throw (<anonymous>)
221
      at rejected (/home/runner/work/_actions/pulumi/actions/76683de37aa44910871ba6cef36557780f2e41d1/webpack:/pulumi-github-action/node_modules/@pulumi/pulumi/automation/cmd.js:19:1)
222
      at processTicksAndRejections (node:internal/process/task_queues:95:5)

@haydentherapper
Copy link
Contributor

@jku we might have to split this PR into two. I think the error is because the role does not yet exist, so the script fails to assign the new role even though it’s created later in the script. Wish pulumi had a way to define dependents…

@jku
Copy link
Member Author

jku commented Feb 8, 2024

@jku we might have to split this PR into two. I think the error is because the role does not yet exist, so the script fails to assign the new role even though it’s created later in the script.

I can do that but

  1. half of it did already get deployed: the custom role now exists, you can see the ghost in the completely unrelated Add Pages publishing to root-signing-staging #398...
  2. The script does things in correct order AFAICT. Either the GH API is not immediately consistent or there is some bug in my code

So an alternative we could try (unless there's a danger I don't see) is to just re-run this deployment: the custom role now seems to exist in GitHub so if the rest of the code is correct, it should now go through)...

EDIT: still failing so there is a problem in my code...

jku added a commit to jku/community that referenced this pull request Feb 8, 2024
This is a revert of half of sigstore#395.

'pulumi up' fails with this:
  422 Role `write-with-bypass` is not available for the
  sigstore/root-signing-staging repository.

I'm not sure why the custom role is not found. Let's revert this
and I will investigate.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
bobcallaway pushed a commit that referenced this pull request Feb 8, 2024
This is a revert of half of #395.

'pulumi up' fails with this:
  422 Role `write-with-bypass` is not available for the
  sigstore/root-signing-staging repository.

I'm not sure why the custom role is not found. Let's revert this
and I will investigate.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
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.

4 participants