Create data source for organization ip allow list #1275
Conversation
|
Hi @kfcampbell, would appreciate your review of this PR to add data source to retrieve an org's IP allow list. Thanks! |
| "github_actions_secrets": dataSourceGithubActionsSecrets(), | ||
| "github_actions_organization_secrets": dataSourceGithubActionsOrganizationSecrets(), |
There was a problem hiding this comment.
sorted the resources
| "github_actions_organization_secrets": dataSourceGithubActionsOrganizationSecrets(), | ||
| "github_organization_ip_allow_list": dataSourceGithubOrganizationIpAllowList(), |
There was a problem hiding this comment.
sorted the resources by moving actions resource to top; then added new the ip allow list resource
|
I'm seeing the following error when running the integration test locally: |
|
Hrmm, will take a look. It was passing for sure before I back merged with main branch today. I saw the commit check pass so thought that meant the back merge went well. |
|
@kfcampbell The test succeeds locally for me. How are you running the test? This is what I do from the root of the project: |
|
Oh, I just noticed the key part in that test result: Stand by, let me set some vars and retest to confirm ⏳ |
|
Yes, that test passes for me locally. |
|
Unfortunately, our integration tests are being run manually at this stage and are not automatically run on PR checks. This is weird...I have different results. When I'm up-to-date with your branch, I set the following environment variables: sh$ env | grep GITHUB
GITHUB_TOKEN=REDACTED
GITHUB_ORGANIZATION=kfcampbell-terraform-provider
sh$ env | grep TF_
TF_ACC=1Then I'll run the test using your same command: FWIW, I have never set or modified IP allowlists for my test organization, which may be why this is failing. |
|
Yep, manually setting an IP and enabling an allowlist organization makes the test pass. What would you prefer to do here? We could merge as-is with the idea that once #1067 is closed, we could come back and fix the test. |
|
@kfcampbell Thanks for confirming. Hrm, I don't have an Enterprise Org that I can test where it doesn't have IP allow lists added but the feature is enabled. ❓ Does the test fail for you if the feature is enabled but no IPs allowed? If it's only failing when the feature isn't enabled, I can try to make the test smarter to check whether it's enabled or not and skip with a message to the user. Or if the test is simply failing because in either case (feature disabled or no IPs allowed) then the result is empty and so the assertion can't be satisified as written, I can see if I can update the assertion to check for empty list. If the feature isn't enabled, I would have expected this query to have failed further upstream before the test has a chance to assert the output.
Although I'd love to get this merged now, it's probably best to wait until the test can pass when the feature is enabled with zero allowed IPs (which could very well be a scenario a customer has, not sure why they'd be using this if that were the case, but best not to break their terraform build just because they have zero IPs). |
|
@kfcampbell you know what, I don't think an org can both have IP allow list on AND have no IPs listed as that would prevent the users from accessing it entirely. So there likely has to be at least one IP. In that case, I think this is good to merge as-is. With the known room for improvement that the test should skip if the feature isn't enabled to explain why the test didn't run. Which essentially me finally coming back around to your original suggestion 😁
|
|
😄 That works for me! I'll merge and release this, and I'm looking forward to checking out any progress on #1067. |
There was a problem hiding this comment.
After #1067 is closed, the test TestAccGithubOrganizationIpAllowListDataSource should be amended to ensure that the organization being tested has the IP allowlist feature enabled.
douglascayers
deleted the
data-source-github-organization-ip-allow-list
branch
* feat: add data source to get org's ip allow list * test: add test for org ip allow list data source * doc: document org ip allow list data source * chore: backmerge from main * chore: sort resources
* feat: add data source to get org's ip allow list * test: add test for org ip allow list data source * doc: document org ip allow list data source * chore: backmerge from main * chore: sort resources
Summary
Allow terraform to retrieve an enterprise cloud organization's IP allow list.
This data source is a precursor to a future PR to add a resource to allow terraform to manage IP allow list entries as requested in #1067.
Changes
Tests
github/data_source_github_organization_ip_allow_list_test.goUsage
Example configuration:
Example output:
{ "resources": [ { "mode": "data", "type": "github_organization_ip_allow_list", "name": "all", "instances": [ { "schema_version": 0, "attributes": { "id": "MDEyOk9yZ2FuaXphdGlvbjg0NDkwOTE4", "ip_allow_list": [ { "allow_list_value": "192.30.252.0/22", "created_at": "2022-06-10T22:58:22Z", "id": "IALE_kwHOBQk6ps4ACyRH", "is_active": true, "name": "GitHub Hooks", "updated_at": "2022-06-10T22:58:22Z" }, { "allow_list_value": "185.199.108.0/22", "created_at": "2022-06-10T22:58:22Z", "id": "IALE_kwHOBQk6ps4ACyRH", "is_active": true, "name": "GitHub Hooks", "updated_at": "2022-06-10T22:58:22Z" } ] }, "sensitive_attributes": [] } ] } ] }