Skip to content
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

Add resource for GitHub integration #311

Merged
merged 14 commits into from
Jun 4, 2020

Conversation

tptee
Copy link
Contributor

@tptee tptee commented May 16, 2020

This PR adds a resource for integrating GitHub into a GitLab project. It replicates the patterns from the Jira and Slack resources and uses the Service API via go-gitlab.

The GitHub integration is a Premium+ feature, so running the acceptance test is tricky–maybe there's a way to omit it from make testacc? Additionally, this go-gitlab PR prevents this resource and its acceptance test from working. When this fix lands upstream (may need a backport because of breaking changes in 0.31.x, I'll update the PR and run the acceptance test in a Silver .com group!

@ghost ghost added the size/XL label May 16, 2020
@ringods
Copy link
Contributor

ringods commented May 20, 2020

@tptee I just merged #304 which takes in go-gitlab v0.31.0. You can rebase your branch now. 😉

@ringods ringods marked this pull request as draft May 20, 2020 09:40
@ghost ghost added size/XXL and removed size/XL labels May 20, 2020
@ringods
Copy link
Contributor

ringods commented May 21, 2020

@tptee for my understanding, is there still a fix needed in go-gitlab to make the acceptance tests for the provider working for your PR? If so, does this mean we have to await the next release of go-gitlab then?

@tptee
Copy link
Contributor Author

tptee commented May 21, 2020

I've pointed to the latest master of go-gitlab in my branch, but ideally we bump to the upcoming official 0.31.1 release from them. The acceptance tests pass for me locally if I provide a namespace from a Silver plan, but fail in acceptance-ce. The GitHub integration is an EE/Silver+ feature, so I think the /service/github endpoint doesn't exist in CE, hence the 404s in the test failure:

 === RUN   TestAccGitlabServiceGithub_basic
##[error]    TestAccGitlabServiceGithub_basic: testing.go:654: Step 0 error: errors during apply:
        
        Error: PUT http://127.0.0.1:8080/api/v4/projects/24/services/github: 404 {error: 404 Not Found}
        
          on /tmp/tf-test168658687/main.tf line 8:
          (source code not available)
        
        
--- FAIL: TestAccGitlabServiceGithub_basic (5.47s)
=== RUN   TestAccGitlabServiceGithub_import
##[error]    TestAccGitlabServiceGithub_import: testing.go:654: Step 0 error: errors during apply:
        
        Error: PUT http://127.0.0.1:8080/api/v4/projects/25/services/github: 404 {error: 404 Not Found}
        
          on /tmp/tf-test931785609/main.tf line 8:
          (source code not available)
        
        
--- FAIL: TestAccGitlabServiceGithub_import (5.43s)

Any guidance for how we can separate EE-only tests from the CE test run?

@ringods
Copy link
Contributor

ringods commented May 22, 2020

@tptee I filed #314 to investigate how the acceptance tests can be divided in the community and the enterprise editions.

@roidelapluie
Copy link
Collaborator

I've pointed to the latest master of go-gitlab in my branch, but ideally we bump to the upcoming official 0.31.1 release from them. The acceptance tests pass for me locally if I provide a namespace from a Silver plan, but fail in acceptance-ce. The GitHub integration is an EE/Silver+ feature, so I think the /service/github endpoint doesn't exist in CE, hence the 404s in the test failure:

 === RUN   TestAccGitlabServiceGithub_basic
##[error]    TestAccGitlabServiceGithub_basic: testing.go:654: Step 0 error: errors during apply:
        
        Error: PUT http://127.0.0.1:8080/api/v4/projects/24/services/github: 404 {error: 404 Not Found}
        
          on /tmp/tf-test168658687/main.tf line 8:
          (source code not available)
        
        
--- FAIL: TestAccGitlabServiceGithub_basic (5.47s)
=== RUN   TestAccGitlabServiceGithub_import
##[error]    TestAccGitlabServiceGithub_import: testing.go:654: Step 0 error: errors during apply:
        
        Error: PUT http://127.0.0.1:8080/api/v4/projects/25/services/github: 404 {error: 404 Not Found}
        
          on /tmp/tf-test931785609/main.tf line 8:
          (source code not available)
        
        
--- FAIL: TestAccGitlabServiceGithub_import (5.43s)

Any guidance for how we can separate EE-only tests from the CE test run?

Use SkipFunc: isRunningInCE.

@tptee tptee force-pushed the resource_gitlab_service_github branch from 19a854d to 4f882f5 Compare May 22, 2020 21:10
@ghost ghost added size/XL dependencies and removed size/XXL labels May 22, 2020
@tptee tptee marked this pull request as ready for review May 22, 2020 21:22
@tptee tptee changed the title [WIP] Add resource for GitHub integration Add resource for GitHub integration May 22, 2020
@tptee
Copy link
Contributor Author

tptee commented May 22, 2020

Ok CE tests pass now–all that's left is to bump to 0.31.1 of go-gitlab once it's released!

I do worry that the EE tests will fail–the SkipFunc fails for me on my machine if I only run the single test (haven't tried running the entire suite, not sure it'll work out-of-the-box on .com or not yet 😄 ):

=== RUN   TestAccGitlabServiceGithub_basic
    TestAccGitlabServiceGithub_basic: testing.go:606: Provider not initialized, unable to get GitLab connection
--- FAIL: TestAccGitlabServiceGithub_basic (0.00s)
=== RUN   TestAccGitlabServiceGithub_import
    TestAccGitlabServiceGithub_import: testing.go:606: Provider not initialized, unable to get GitLab connection
--- FAIL: TestAccGitlabServiceGithub_import (0.00s)
FAIL
FAIL    github.com/terraform-providers/terraform-provider-gitlab/gitlab 0.036s
FAIL

This is with GITLAB_TOKEN=redacted TF_ACC=1 go test -v ./gitlab -run 'TestAccGitlabServiceGithub' -timeout 40m -count 1. Any ideas on how to run a single acceptance test that depends on testAccProvider being initialized? I'm sure I'm missing something obvious!

@tptee
Copy link
Contributor Author

tptee commented May 22, 2020

I ran the whole suite on a .com Silver plan group and the new test passed!

@ghost ghost added size/XXL and removed size/XL labels May 26, 2020
@tptee
Copy link
Contributor Author

tptee commented May 26, 2020

Upgraded to v0.32.0 of go-gitlab and reconfirmed the tests pass! I think this is ready to go as long as the EE tests pass for maintainers!

@tptee
Copy link
Contributor Author

tptee commented May 26, 2020

Looks like an unrelated failure from the go-gitlab upgrade:

 ##[error]    TestAccGitlabProject_basic: testing.go:654: Step 1 error: Check failed: Check 2/2 error: 1 error occurred:
        	* Check 10/28 error: attribute container_registry_enabled expected "false" received "true"

I'll take a peek at the changes in that release to see what's up.

@ringods
Copy link
Contributor

ringods commented Jun 2, 2020

@tptee test probably fails due to this:

xanzy/go-gitlab@12ce540#r39607378

@ringods
Copy link
Contributor

ringods commented Jun 3, 2020

@tptee I need to ask you to rebase once more. Branch master already contains go-gitlab v0.32.1 which fixes the failing test, so please make sure you do not revert to v0.32.0.

@tptee tptee force-pushed the resource_gitlab_service_github branch from 5b74c5a to 488cd4c Compare June 3, 2020 16:18
@ghost ghost added size/XL and removed size/XXL labels Jun 3, 2020
@tptee
Copy link
Contributor Author

tptee commented Jun 3, 2020

Tests passed locally for me after rebase!

@ringods
Copy link
Contributor

ringods commented Jun 3, 2020

@tptee Without documentation, this new resource won't show up on the Terraform website. Can you still add it to the website/docs/r folder? You can start of with a copy of one of the existing service_...html.markdown files.

@tptee
Copy link
Contributor Author

tptee commented Jun 3, 2020

oh whoops, on it!

@ghost ghost added the documentation label Jun 3, 2020
@tptee
Copy link
Contributor Author

tptee commented Jun 3, 2020

Done! Sorry for missing that!

@ringods
Copy link
Contributor

ringods commented Jun 3, 2020

@tptee and I forgot to mention that you also have to add the resource to the website sidebar here: website/gitlab.erb. Sorry for that. 😉

@tptee
Copy link
Contributor Author

tptee commented Jun 3, 2020

No worries! on it!

@tptee
Copy link
Contributor Author

tptee commented Jun 3, 2020

Done! Thanks for your patience 🙇

@ringods
Copy link
Contributor

ringods commented Jun 4, 2020

@tptee more in reverse. Thank you for all the work in this contribution.

@ringods ringods merged commit 27c1c28 into gitlabhq:master Jun 4, 2020
ahmet2mir pushed a commit to ahmet2mir/terraform-provider-gitlab that referenced this pull request Sep 15, 2020
…thub

Add resource for GitHub integration
@github-actions github-actions bot locked and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

4 participants