-
Notifications
You must be signed in to change notification settings - Fork 778
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
feat: support repository level custom_property resource and custom_properties datasource #2316
Merged
kfcampbell
merged 40 commits into
integrations:main
from
felixlut:repository-custom-properties
Jan 17, 2025
Merged
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
a7e562c
add octokit sdk client
af0245a
stash half working solution
cdd112a
add repository custom properties data source
f9633fb
break out custom props parsing logic to its own function
5e0894d
use background ctx
2accb56
format provider.go
b3f44ca
fix error msg to include repoName instead of its pointer
98947a3
use type switch instead of if else
2125cab
fix linting errors
ec2755e
restructure datasource to take a property name and call it github_rep…
1e88e7a
formatting
31f9a2b
rename file to match datasource name
5ed47ba
Merge branch 'main' into repository-custom-properties
felixlut 697c43c
remove go-sdk in favor of go-github
9933170
implement data_source_github_repository_custom_property with go-github
a7aefae
implement resource_github_repository_custom_property
ce87c42
update descriptions
6b76479
remove custom_property resource in favour of custom_propertIES one
d9ad9a9
add custom_property resource to provider.go
7982a1a
formatting
7366a88
add tests for repository_custom_property
fe5add1
update description of test
0dbd149
add tests for each custom_property type
d9f13fd
rollback repo changes
7615100
add tests for custom_property datasource
c72ce22
formatting
bc93a88
Merge branch 'main' into repository-custom-properties
felixlut d298e07
breakout parsing custom_property_value as a string slice to its own f…
8eff6f4
Merge branch 'main' into repository-custom-properties
felixlut 3deb3ed
bump go-github to v66 for new files
felixlut 31ac7db
add property_type as a required attribute
felixlut 10bb6f0
flip datasource to use typeList instead of typeSet
felixlut 1e3ba1d
refactor tests for data source to use property_type
felixlut 5be94bd
Update data_source_github_repository_custom_properties.go
felixlut d9974e6
cleanup incorrect comments
felixlut add24a8
remove old comment
felixlut 46c1846
add docs
felixlut 15b61e7
fix typo
felixlut 2b169f1
Merge branch 'main' into repository-custom-properties
felixlut 1b6bfaa
Merge branch 'main' into repository-custom-properties
kfcampbell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
remove custom_property resource in favour of custom_propertIES one
- Loading branch information
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
package github | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
|
||
"github.com/google/go-github/v65/github" | ||
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" | ||
) | ||
|
||
func dataSourceGithubRepositoryCustomProperties() *schema.Resource { | ||
return &schema.Resource{ | ||
Read: dataSourceGithubOrgaRepositoryCustomProperties, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"repository": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
Description: "Name of the repository which the custom properties should be on.", | ||
}, | ||
"property": { | ||
Type: schema.TypeSet, | ||
Computed: true, | ||
Description: "List of custom properties", | ||
Elem: &schema.Resource{ | ||
Schema: map[string]*schema.Schema{ | ||
"property_name": { | ||
Type: schema.TypeString, | ||
Computed: true, | ||
Description: "Name of the custom property.", | ||
}, | ||
"property_value": { | ||
Type: schema.TypeSet, | ||
Computed: true, | ||
Description: "Value of the custom property.", | ||
Elem: &schema.Schema{ | ||
Type: schema.TypeString, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func dataSourceGithubOrgaRepositoryCustomProperties(d *schema.ResourceData, meta interface{}) error { | ||
|
||
client := meta.(*Owner).v3client | ||
ctx := context.Background() | ||
|
||
owner := meta.(*Owner).name | ||
|
||
repoName := d.Get("repository").(string) | ||
|
||
allCustomProperties, _, err := client.Repositories.GetAllCustomPropertyValues(ctx, owner, repoName) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
results, err := flattenRepositoryCustomProperties(allCustomProperties) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
d.SetId(buildTwoPartID(owner, repoName)) | ||
d.Set("repository", repoName) | ||
d.Set("property", results) | ||
|
||
return nil | ||
} | ||
|
||
func flattenRepositoryCustomProperties(customProperties []*github.CustomPropertyValue) ([]interface{}, error) { | ||
|
||
results := make([]interface{}, 0) | ||
for _, prop := range customProperties { | ||
result := make(map[string]interface{}) | ||
|
||
result["property_name"] = prop.PropertyName | ||
|
||
switch value := prop.Value.(type) { | ||
case string: | ||
result["property_value"] = []string{value} | ||
case []string: | ||
result["property_value"] = value | ||
default: | ||
return nil, fmt.Errorf("custom property value couldn't be parsed as a string or a list of strings: %s", value) | ||
} | ||
|
||
results = append(results, result) | ||
} | ||
|
||
return results, nil | ||
} |
This file was deleted.
Oops, something went wrong.
This file contains 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way I see most people using this resource is to:
owner
property equals Y, do Z to the repo if theenvironment
property equals ...This means that it should be as easy as possible to get the value of a specific property. IMO that would be best achieved by using a
TypeMap
. Unfortunately that is not supported by v2 of the terraform-plugin-sdk (see thread).The choice is then between
TypeList
orTypeSet
. Reading the docs the latter sounds more correct to me, as the order of the custom properties does not matter. We've seen issues regarding this in this provider in the past, eg in #1950, that should be avoided by usingTypeSet
.It's worth noting that there are some other implications of using
TypeSet
though, namely that they are non-indexable. This common pattern will for example not work, and throw the error below:That does not mean that it's impossible to fetch the value of a custom property though. Either of the two snippets below does work, and are IMO more readable, but I think I'd be amiss if I didn't mention this.
I personally lean towards using a
TypeSet
to avoid possible weird diffs, but I don't feel strongly about it. If anyone have any opinions here I'm all ears 😄There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@felixlut have you considered also adding a
github_repository_custom_property
data source to access a singular property? It would allow for more direct access to properties at the cost of increased API usage.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have, and nothing stops future PR:s for doing just that. It probably should exist, but you can't do everything in 1 PR 😅
The actual implementation would be to simply filter it after this call