Skip to content

Commit 0271796

Browse files
authored
fix: Prevent enterprise org taint on SAML enforcement error (#3026)
* fix: Prevent enterprise org taint on SAML enforcement error When creating an enterprise organization in an EMU environment, the REST API call to set description/display_name fails with a SAML enforcement error until the PAT is authorized for the new org. Previously this would taint the resource, causing Terraform to destroy and recreate the org on the next apply. This fix: - Catches SAML enforcement errors in Create and Update functions - Clears description/display_name from state on create, resets to previous values on update, so state reflects reality and next plan shows drift - Returns success instead of error to prevent tainting - Logs a warning instructing the user to authorize the PAT and re-apply Fixes: #1914 # Conflicts: # github/resource_github_enterprise_organization.go * Update github/resource_github_enterprise_organization_test.go
1 parent fbcab61 commit 0271796

File tree

2 files changed

+104
-5
lines changed

2 files changed

+104
-5
lines changed

github/resource_github_enterprise_organization.go

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,19 @@ import (
1212
"github.com/shurcooL/githubv4"
1313
)
1414

15+
func isSAMLEnforcementError(err error) bool {
16+
if err == nil {
17+
return false
18+
}
19+
var ghErr *github.ErrorResponse
20+
if errors.As(err, &ghErr) {
21+
return ghErr.Response != nil &&
22+
ghErr.Response.StatusCode == 403 &&
23+
strings.Contains(ghErr.Message, "SAML enforcement")
24+
}
25+
return strings.Contains(err.Error(), "Resource protected by organization SAML enforcement")
26+
}
27+
1528
func resourceGithubEnterpriseOrganization() *schema.Resource {
1629
return &schema.Resource{
1730
Create: resourceGithubEnterpriseOrganizationCreate,
@@ -128,7 +141,17 @@ func resourceGithubEnterpriseOrganizationCreate(data *schema.ResourceData, meta
128141
Name: github.Ptr(displayName),
129142
},
130143
)
131-
return err
144+
if err != nil {
145+
if isSAMLEnforcementError(err) {
146+
// The org was created but we can't set description/display_name until the PAT is authorized.
147+
// Clear them from state so next plan will show drift and retry after PAT authorization.
148+
log.Printf("[WARN] Organization %q created but could not set description/display_name due to SAML enforcement. Authorize the PAT and run apply again.", data.Get("name").(string))
149+
_ = data.Set("description", "")
150+
_ = data.Set("display_name", "")
151+
return nil
152+
}
153+
return err
154+
}
132155
}
133156
return nil
134157
}
@@ -301,10 +324,18 @@ func updateDescription(ctx context.Context, data *schema.ResourceData, v3 *githu
301324
ctx,
302325
orgName,
303326
&github.Organization{
304-
Description: github.Ptr(data.Get("description").(string)),
327+
Description: github.Ptr(newDesc),
305328
},
306329
)
307-
return err
330+
if err != nil {
331+
if isSAMLEnforcementError(err) {
332+
// Reset state to old value so next plan shows drift
333+
log.Printf("[WARN] Could not update description for %q due to SAML enforcement. Authorize the PAT and run apply again.", orgName)
334+
_ = data.Set("description", oldDesc)
335+
return nil
336+
}
337+
return err
338+
}
308339
}
309340
return nil
310341
}
@@ -318,10 +349,18 @@ func updateDisplayName(ctx context.Context, data *schema.ResourceData, v4 *githu
318349
ctx,
319350
orgName,
320351
&github.Organization{
321-
Name: github.Ptr(data.Get("display_name").(string)),
352+
Name: github.Ptr(newDisplayName),
322353
},
323354
)
324-
return err
355+
if err != nil {
356+
if isSAMLEnforcementError(err) {
357+
// Reset state to old value so next plan shows drift
358+
log.Printf("[WARN] Could not update display_name for %q due to SAML enforcement. Authorize the PAT and run apply again.", orgName)
359+
_ = data.Set("display_name", oldDisplayName)
360+
return nil
361+
}
362+
return err
363+
}
325364
}
326365
return nil
327366
}

github/resource_github_enterprise_organization_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,75 @@
11
package github
22

33
import (
4+
"errors"
45
"fmt"
6+
"net/http"
57
"regexp"
68
"strings"
79
"testing"
810

11+
"github.com/google/go-github/v81/github"
912
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
1013
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
1114
)
1215

16+
func TestIsSAMLEnforcementError(t *testing.T) {
17+
tests := []struct {
18+
name string
19+
err error
20+
expected bool
21+
}{
22+
{
23+
name: "nil error",
24+
err: nil,
25+
expected: false,
26+
},
27+
{
28+
name: "GitHub ErrorResponse with SAML enforcement",
29+
err: &github.ErrorResponse{
30+
Response: &http.Response{StatusCode: 403},
31+
Message: "Resource protected by organization SAML enforcement. You must grant your Personal Access token access to this organization.",
32+
},
33+
expected: true,
34+
},
35+
{
36+
name: "GitHub ErrorResponse 403 without SAML message",
37+
err: &github.ErrorResponse{
38+
Response: &http.Response{StatusCode: 403},
39+
Message: "Forbidden",
40+
},
41+
expected: false,
42+
},
43+
{
44+
name: "GitHub ErrorResponse 404",
45+
err: &github.ErrorResponse{
46+
Response: &http.Response{StatusCode: 404},
47+
Message: "Not Found",
48+
},
49+
expected: false,
50+
},
51+
{
52+
name: "plain error with SAML enforcement message",
53+
err: errors.New("Resource protected by organization SAML enforcement"),
54+
expected: true,
55+
},
56+
{
57+
name: "plain error without SAML message",
58+
err: errors.New("some other error"),
59+
expected: false,
60+
},
61+
}
62+
63+
for _, tc := range tests {
64+
t.Run(tc.name, func(t *testing.T) {
65+
result := isSAMLEnforcementError(tc.err)
66+
if result != tc.expected {
67+
t.Errorf("isSAMLEnforcementError(%v) = %v, want %v", tc.err, result, tc.expected)
68+
}
69+
})
70+
}
71+
}
72+
1373
func TestAccGithubEnterpriseOrganization(t *testing.T) {
1474
t.Run("creates and updates an enterprise organization without error", func(t *testing.T) {
1575
randomID := acctest.RandStringFromCharSet(5, acctest.CharSetAlphaNum)

0 commit comments

Comments
 (0)