feat: Add EnterpriseOwner as bypass actor type for rulesets#3487
Open
nitinjain999 wants to merge 7 commits into
Open
feat: Add EnterpriseOwner as bypass actor type for rulesets#3487nitinjain999 wants to merge 7 commits into
nitinjain999 wants to merge 7 commits into
Conversation
- Add EnterpriseOwner to the ValidateDiagFunc allowlist for actor_type in both organization and repository ruleset resources - Update actor_id descriptions to clarify it should be omitted for OrganizationAdmin, EnterpriseOwner, and DeployKey (API ignores it) - Remove actor_id=1 from OrganizationAdmin bypass_actors in tests since the GitHub API ignores the value for that actor type - Add separate enterprise-gated acceptance tests for EnterpriseOwner bypass actor using ConfigStateChecks (skipUnlessEnterprise) - Update docs/resources/ (new docs system) for both resources Closes integrations#3210
|
👋 Hi, and thank you for this contribution! This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can. You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions. 🤖 This is an automated message. |
1 task
5 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds EnterpriseOwner to the allowed actor_type values for bypass_actors in both the github_organization_ruleset and github_repository_ruleset resources. Previously, configuring an Enterprise Owner as a bypass actor failed plan-time validation even though the GitHub API supports it (closes #3210, supersedes #3211). The change is additive and backward-compatible (a new accepted enum value, no schema-shape change), and also clarifies that actor_id is ignored by the API for ID-less actor types (OrganizationAdmin, EnterpriseOwner, DeployKey).
Changes:
- Add
EnterpriseOwnerto theactor_typeStringInSlicevalidation and updateactor_id/actor_typedescriptions in both ruleset resources. - Add enterprise-gated acceptance tests (
skipUnlessEnterprise,ConfigStateChecks) covering anEnterpriseOwnerbypass actor, and drop the no-opactor_id = 1from existingOrganizationAdmintest blocks/assertions. - Update the rendered docs under
docs/resources/(but not the source templates — see finding below).
HIGH — Documentation edits will be reverted by CI
docs/resources/organization_ruleset.mdanddocs/resources/repository_ruleset.mdare auto-generated. Thebypass_actorsnarrative is hand-written intemplates/resources/*_ruleset.md.tmpl, not rendered from the schema. CI runsmake checkdocs(.github/workflows/ci.yaml:88), which regenerates docs and fails on any diff. Since the templates were not updated, the doc edits will be reverted and the docs job will fail. The same edits must be applied to the correspondingtemplates/resources/*_ruleset.md.tmplfiles.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
github/resource_github_organization_ruleset.go |
Adds EnterpriseOwner to actor_type allowlist; updates actor_id/actor_type descriptions. |
github/resource_github_repository_ruleset.go |
Same allowlist/description update for the repository ruleset. |
github/resource_github_organization_ruleset_test.go |
New enterprise-gated EnterpriseOwner test; removes no-op actor_id = 1 from OrganizationAdmin configs/assertions. |
github/resource_github_repository_ruleset_test.go |
New enterprise-gated EnterpriseOwner test using ConfigStateChecks. |
docs/resources/organization_ruleset.md |
Edits generated docs directly (should be made in the template). |
docs/resources/repository_ruleset.md |
Edits generated docs directly (should be made in the template). |
Collaborator
|
@nitinjain999 are you able to address change requests to this today and tomorrow? We want to release v1.13.0 this week |
…ctor_id description - Add EnterpriseOwner to actor_type allowed values in both ruleset templates - Clarify that actor_id is ignored by the GitHub API for OrganizationAdmin, EnterpriseOwner, and DeployKey - Remove stale OrganizationAdmin -> 1 mapping note Fixes docs CI failure caused by editing auto-generated docs/ files directly instead of the source templates/resources/*.md.tmpl files.
…r types Resolves conflicts between this branch (EnterpriseOwner) and upstream main (User, merged via integrations#3438). Both actor types are now supported: - ValidateDiagFunc includes EnterpriseOwner and User - actor_id description covers Integration, User, and the ID-less types - Template and docs are in sync
Contributor
Author
@deiga : Done |
…nd docs DeployKey was already in the schema validation but missing from the docs actor_type description. Regenerated docs from template.
| Optional: true, | ||
| Default: nil, | ||
| Description: "The ID of the actor that can bypass a ruleset. When `actor_type` is `OrganizationAdmin`, this should be set to `1`. Some resources such as DeployKey do not have an ID and this should be omitted.", | ||
| Description: "The ID of the actor that can bypass a ruleset. Some actor types such as `OrganizationAdmin`, `EnterpriseOwner`, and `DeployKey` do not have an ID — this argument should not be set in those cases as the GitHub API will ignore it.", |
Comment on lines
74
to
76
| bypass_actors { | ||
| actor_id = 1 | ||
| actor_type = "OrganizationAdmin" | ||
| bypass_mode = "always" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3210
Supersedes #3211
Picking up from @jackmtpt who did the initial work in #3211.
The issue is straightforward —
EnterpriseOwnerwas missing from theactor_typevalidation list in bothgithub_organization_rulesetandgithub_repository_ruleset, so using it would fail with a validation error even though the GitHub API supports it fine.Changes:
EnterpriseOwnerto theactor_typeallowlist in both resourcesactor_iddescription to note it should be left unset forOrganizationAdmin,EnterpriseOwner, andDeployKey— the API ignores it for those typesactor_id = 1fromOrganizationAdminblocks in existing tests (flagged by @deiga — same deal, API ignores it)EnterpriseOwnerbypass, enterprise-gated withskipUnlessEnterpriseand usingConfigStateChecksdocs/resources/—website/docs/no longer exists on mainTested against a live enterprise org. Create, plan (no drift on second run), destroy all worked. One thing to flag: the API returns
actor_id: 0forEnterpriseOwneron read, so if you exposebypass_actorsas an output you'll see anull → 0change on the first plan after apply. Resource itself has no diff though — same behaviour asOrganizationAdmin.