Skip to content

Commit ed8d940

Browse files
yumafuucharmcrush
andauthored
feat: make repository attribute compatible with GitHub provider format (#11)
* feat: make repository attribute compatible with GitHub provider format Change repository attribute from 'owner/repo' format to 'repo-name' format to match GitHub official provider conventions. Owner is now configured at the provider level via 'owner' attribute or GITHUB_OWNER environment variable. This breaking change improves consistency with the official GitHub provider and simplifies resource configuration. 💖 Generated with Crush Co-Authored-By: Crush <[email protected]> * fmt --------- Co-authored-by: Crush <[email protected]>
1 parent cd62cf8 commit ed8d940

File tree

4 files changed

+34
-74
lines changed

4 files changed

+34
-74
lines changed

internal/githubclient/client.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,27 +19,28 @@ import (
1919

2020
type Client struct {
2121
*github.Client
22+
Owner string
2223
}
2324

24-
func NewClient(token, baseURL string) *Client {
25+
func NewClient(token, baseURL, owner string) *Client {
2526
var tc *http.Client
2627
if token != "" {
2728
tc = github.NewClient(nil).WithAuthToken(token).Client()
2829
}
2930

3031
client, _ := github.NewClient(tc).WithEnterpriseURLs(baseURL, baseURL)
31-
return &Client{client}
32+
return &Client{client, owner}
3233
}
3334

34-
func NewClientWithApp(appID, installationID, pemFile, baseURL string) *Client {
35+
func NewClientWithApp(appID, installationID, pemFile, baseURL, owner string) *Client {
3536
token, err := GenerateOAuthTokenFromApp(baseURL, appID, installationID, pemFile)
3637
if err != nil {
3738
return nil
3839
}
3940

4041
tc := github.NewClient(nil).WithAuthToken(token).Client()
4142
client, _ := github.NewClient(tc).WithEnterpriseURLs(baseURL, baseURL)
42-
return &Client{client}
43+
return &Client{client, owner}
4344
}
4445

4546
func GenerateOAuthTokenFromApp(baseURL, appID, appInstallationID, pemData string) (string, error) {

internal/provider/provider.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,18 @@ func (p *kwgithubProvider) Configure(ctx context.Context, req provider.Configure
9595
baseURL = envBaseURL
9696
}
9797

98+
owner := ""
99+
if !config.Owner.IsNull() && !config.Owner.IsUnknown() {
100+
owner = config.Owner.ValueString()
101+
} else if envOwner := os.Getenv("GITHUB_OWNER"); envOwner != "" {
102+
owner = envOwner
103+
}
104+
105+
if owner == "" {
106+
resp.Diagnostics.AddError("Missing owner", "owner must be configured either in provider configuration or via GITHUB_OWNER environment variable")
107+
return
108+
}
109+
98110
var token string
99111
var client *githubclient.Client
100112

@@ -131,7 +143,7 @@ func (p *kwgithubProvider) Configure(ctx context.Context, req provider.Configure
131143
}
132144

133145
pemFile = strings.Replace(pemFile, `\n`, "\n", -1)
134-
client = githubclient.NewClientWithApp(appID, installationID, pemFile, baseURL)
146+
client = githubclient.NewClientWithApp(appID, installationID, pemFile, baseURL, owner)
135147
if client == nil {
136148
resp.Diagnostics.AddError(
137149
"Failed to create GitHub App client",
@@ -150,7 +162,7 @@ func (p *kwgithubProvider) Configure(ctx context.Context, req provider.Configure
150162
resp.Diagnostics.AddError("Missing authentication", "Either token or app_auth must be configured")
151163
return
152164
}
153-
client = githubclient.NewClient(token, baseURL)
165+
client = githubclient.NewClient(token, baseURL, owner)
154166
}
155167

156168
resp.ResourceData = client

internal/provider/resource_ruleset_merge_methods.go

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func (r *rulesetAllowedMergeMethodsResource) Schema(
4949
Attributes: map[string]schema.Attribute{
5050
"repository": schema.StringAttribute{
5151
Required: true,
52-
Description: "The name of the repository (e.g., 'owner/repo').",
52+
Description: "The name of the repository (e.g., 'repo-name').",
5353
},
5454
"ruleset_id": schema.StringAttribute{
5555
Required: true,
@@ -121,11 +121,8 @@ func (r *rulesetAllowedMergeMethodsResource) Read(
121121
return
122122
}
123123

124-
owner, repo, err := parseRepo(state.Repository.ValueString())
125-
if err != nil {
126-
resp.Diagnostics.AddError("Invalid repository format", err.Error())
127-
return
128-
}
124+
owner := r.client.Owner
125+
repo := state.Repository.ValueString()
129126

130127
rulesetID, err := parseID(state.RulesetID.ValueString())
131128
if err != nil {
@@ -150,7 +147,7 @@ func (r *rulesetAllowedMergeMethodsResource) Read(
150147
expectedMethods := extractMethodsFromSet(state.AllowedMergeMethods)
151148
if !methodsEqual(currentMethods, expectedMethods) {
152149
// Methods have been reset by GitHub, restore them
153-
err = r.restoreMergeMethods(ctx, owner, repo, rulesetID, expectedMethods)
150+
err = r.restoreMergeMethods(ctx, repo, rulesetID, expectedMethods)
154151
if err != nil {
155152
resp.Diagnostics.AddWarning(
156153
"Merge methods were reset",
@@ -202,11 +199,8 @@ func (r *rulesetAllowedMergeMethodsResource) Delete(
202199
return
203200
}
204201

205-
owner, repo, err := parseRepo(state.Repository.ValueString())
206-
if err != nil {
207-
resp.Diagnostics.AddError("Invalid repository format", err.Error())
208-
return
209-
}
202+
owner := r.client.Owner
203+
repo := state.Repository.ValueString()
210204

211205
rulesetID, err := parseID(state.RulesetID.ValueString())
212206
if err != nil {
@@ -251,7 +245,7 @@ func (r *rulesetAllowedMergeMethodsResource) ImportState(
251245
if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
252246
resp.Diagnostics.AddError(
253247
"Unexpected Import Identifier",
254-
fmt.Sprintf("Expected import identifier with format: owner/repo:ruleset_id. Got: %q", req.ID),
248+
fmt.Sprintf("Expected import identifier with format: repo:ruleset_id. Got: %q", req.ID),
255249
)
256250
return
257251
}
@@ -264,10 +258,8 @@ func (r *rulesetAllowedMergeMethodsResource) upsert(
264258
ctx context.Context,
265259
plan *rulesetAllowedMergeMethodsResourceModel,
266260
) error {
267-
owner, repo, err := parseRepo(plan.Repository.ValueString())
268-
if err != nil {
269-
return err
270-
}
261+
owner := r.client.Owner
262+
repo := plan.Repository.ValueString()
271263

272264
rulesetID, err := parseID(plan.RulesetID.ValueString())
273265
if err != nil {
@@ -297,14 +289,6 @@ func (r *rulesetAllowedMergeMethodsResource) upsert(
297289
return err
298290
}
299291

300-
func parseRepo(repo string) (string, string, error) {
301-
parts := strings.Split(repo, "/")
302-
if len(parts) != 2 {
303-
return "", "", fmt.Errorf("invalid repository format")
304-
}
305-
return parts[0], parts[1], nil
306-
}
307-
308292
func parseID(id string) (int64, error) {
309293
var i int64
310294
n, err := fmt.Sscanf(id, "%d", &i)
@@ -374,12 +358,12 @@ func methodsEqual(a, b []string) bool {
374358
// restoreMergeMethods restores the merge methods to the expected values
375359
func (r *rulesetAllowedMergeMethodsResource) restoreMergeMethods(
376360
ctx context.Context,
377-
owner, repo string,
361+
repo string,
378362
rulesetID int64,
379363
expectedMethods []string,
380364
) error {
381365
plan := &rulesetAllowedMergeMethodsResourceModel{
382-
Repository: types.StringValue(fmt.Sprintf("%s/%s", owner, repo)),
366+
Repository: types.StringValue(repo),
383367
RulesetID: types.StringValue(fmt.Sprintf("%d", rulesetID)),
384368
AllowedMergeMethods: convertToSet(expectedMethods),
385369
}

internal/provider/resource_ruleset_merge_methods_test.go

Lines changed: 4 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ func TestResourceRulesetAllowedMergeMethodsWithMock(t *testing.T) {
5858
// Test upsert function directly
5959
t.Run("Upsert", func(t *testing.T) {
6060
resource := &rulesetAllowedMergeMethodsResource{
61-
client: &githubclient.Client{Client: client},
61+
client: &githubclient.Client{Client: client, Owner: "owner"},
6262
}
6363

6464
plan := &rulesetAllowedMergeMethodsResourceModel{
65-
Repository: types.StringValue("owner/repo"),
65+
Repository: types.StringValue("repo"),
6666
RulesetID: types.StringValue("123"),
6767
AllowedMergeMethods: convertToSet([]string{"merge"}),
6868
}
@@ -86,13 +86,13 @@ func TestResourceRulesetAllowedMergeMethodsWithMock(t *testing.T) {
8686
getCalled, putCalled = false, false
8787

8888
resource := &rulesetAllowedMergeMethodsResource{
89-
client: &githubclient.Client{Client: client},
89+
client: &githubclient.Client{Client: client, Owner: "owner"},
9090
}
9191

9292
// Test the delete logic by calling the same upsert function
9393
// that Delete would call internally (with default methods)
9494
plan := &rulesetAllowedMergeMethodsResourceModel{
95-
Repository: types.StringValue("owner/repo"),
95+
Repository: types.StringValue("repo"),
9696
RulesetID: types.StringValue("123"),
9797
AllowedMergeMethods: convertToSet([]string{"merge", "squash", "rebase"}), // default methods
9898
}
@@ -143,43 +143,6 @@ func TestResourceRulesetAllowedMergeMethodsWithMock(t *testing.T) {
143143
}
144144

145145
// Unit tests for helper functions
146-
func TestParseRepo(t *testing.T) {
147-
tests := []struct {
148-
input string
149-
expectedOwner string
150-
expectedRepo string
151-
expectError bool
152-
}{
153-
{"owner/repo", "owner", "repo", false},
154-
{"github/terraform-provider", "github", "terraform-provider", false},
155-
{"invalid", "", "", true},
156-
{"owner/repo/extra", "", "", true},
157-
{"", "", "", true},
158-
}
159-
160-
for _, test := range tests {
161-
t.Run(test.input, func(t *testing.T) {
162-
owner, repo, err := parseRepo(test.input)
163-
164-
if test.expectError {
165-
if err == nil {
166-
t.Errorf("Expected error for input %q, but got none", test.input)
167-
}
168-
} else {
169-
if err != nil {
170-
t.Errorf("Unexpected error for input %q: %v", test.input, err)
171-
}
172-
if owner != test.expectedOwner {
173-
t.Errorf("Expected owner %q, got %q", test.expectedOwner, owner)
174-
}
175-
if repo != test.expectedRepo {
176-
t.Errorf("Expected repo %q, got %q", test.expectedRepo, repo)
177-
}
178-
}
179-
})
180-
}
181-
}
182-
183146
func TestParseID(t *testing.T) {
184147
tests := []struct {
185148
input string

0 commit comments

Comments
 (0)