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

Adding support for project templates #277

Merged
merged 1 commit into from
Sep 21, 2020

Conversation

ahmet2mir
Copy link
Contributor

@ahmet2mir ahmet2mir commented Mar 12, 2020

Hello,

I re-use the same test than initializereadme (no state).

use_custom_template, template_project_id and group_with_project_templates_id are EE features
template_name is EE and CE feature.

I see that Gitlab will took probably this repo control #376 so they probably rework all tests to split EE and CE.
We could use tags with go test but i'm not confortable with that and we have to change everywhere

So to simplify EE tests, I added an environment variable GITLAB_EE, when we enter in this test, it's skipped when not EE
(use SkipFunc: isRunningInCE)

on CE

=== RUN   TestAccGitlabProject_templateNameCustom
--- PASS: TestAccGitlabProject_templateNameCustom (0.00s)
=== RUN   TestAccGitlabProject_templateProjectID
--- PASS: TestAccGitlabProject_templateProjectID (0.00s)

In other hand, in start-gitlab, via rails, I:

  • create a group
  • create a template repo (simple git repo)
  • declare the group as a template in admin settings

Thoses lines works with CE too (so I don't remove them, if in future Gitlab move thoses EE features to CE, we just have to remove the GITLAB_EE SkipFunc in tests)

And also migrate to go gitlab 0.37.0

Copy link
Contributor

@sfang97 sfang97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@nicholasklick
Copy link
Collaborator

@armsnyder @roidelapluie if this is looking good can we get it merged?

@sfang97
Copy link
Contributor

sfang97 commented Sep 14, 2020

@ahmet2mir This needed a rebase so I went ahead and kept both changes (testAccGitlabProjectConfigTemplateName from this PR and testAccGitlabProjectConfigImportURL from master), WDYT?

@ahmet2mir
Copy link
Contributor Author

ahmet2mir commented Sep 14, 2020 via email

@sfang97
Copy link
Contributor

sfang97 commented Sep 14, 2020

The branch needed a rebase so I went ahead and rebased it, there was a small conflict that was easy to resolve (I kept both changes), which created a bunch of errors so I want to revert my merge. I'm getting this error though: " ! [remote rejected] project-template-support -> project-template-support (refusing to allow an OAuth App to create or update workflow .github/workflows/acceptance-tests.yml without workflow scope)
error: failed to push some refs to 'https://github.com/ahmet2mir/terraform-provider-gitlab'"
@ahmet2mir or someone else, could you revert my b6d5e38 commit? Sorry for messing with your branch 😅

@ahmet2mir ahmet2mir force-pushed the project-template-support branch 2 times, most recently from 5dfd9a6 to dc76c5b Compare September 15, 2020 08:59
@ahmet2mir
Copy link
Contributor Author

ahmet2mir commented Sep 15, 2020

I will recreate, too much conflicts and my changes are not so big. I did a push force to remove the binary from history and now we have too much commits.

@ahmet2mir ahmet2mir closed this Sep 15, 2020
@ahmet2mir ahmet2mir mentioned this pull request Sep 15, 2020
@ahmet2mir
Copy link
Contributor Author

ahmet2mir commented Sep 15, 2020

Found how to test EE features, see PR comment.

@ahmet2mir ahmet2mir reopened this Sep 15, 2020
@sfang97
Copy link
Contributor

sfang97 commented Sep 15, 2020

I will recreate, too much conflicts and my changes are not so big. I did a push force to remove the binary from history and now we have too much commits.

@ahmet2mir Thanks for reopening, sorry again for messing up your branch!

@armsnyder
Copy link
Collaborator

armsnyder commented Sep 15, 2020

@ahmet2mir Do you need the new GITLAB_EE variable? In other EE-only tests we use SkipFunc: isRunningInCE as in here.

Also, I don't think we want to add any test-specific setup to the start-gitlab.sh file, because the tests should be able to run on existing GitLab instances. Can you create the template in the test code? TestAccGitlabProject_importURL is an example of how to add some custom setup logic.

@ahmet2mir
Copy link
Contributor Author

ahmet2mir commented Sep 16, 2020

@armsnyder I don't need the env var, SkipFunc: isRunningInCE is what I missed. thanks.

For start-gitlab.sh I didn't found another way to achieve this. This setting is defined in EE only and not available via API (or don't found it).

It's the param custom_project_templates_group_id, found on WEB https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/app/models/ee/application_setting.rb#L108 but not on API

I could only create the group, the project myrails based on rails, get the id and send to the tests but I can't change EE settings to declare the group as template.

client := testAccProvider.Meta().(*gitlab.Client)

// Create a base group for importing.
templateGroup, _, err := client.Groups.CreateGroup(&gitlab.CreateGroupOptions{
	Name:       gitlab.String(fmt.Sprintf("terraform-%d", rInt)),
	Path:       gitlab.String(fmt.Sprintf("terraform-%d", rInt)),
	Visibility: gitlab.Visibility(gitlab.PublicVisibility),
})
if err != nil {
	t.Fatalf("failed to create group template: %v", err)
}

// Create a template project based on rails called myrails.
baseProject, _, err := client.Projects.CreateProject(&gitlab.CreateProjectOptions{
	Name:         gitlab.String("myrails"),
	Visibility:   gitlab.Visibility(gitlab.PublicVisibility),
	TemplateName: gitlab.String("rails"),
})
if err != nil {
	t.Fatalf("failed to create template project: %v", err)
}

// Declare group as instance level template group
// not possible

Copy link
Contributor

@sfang97 sfang97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this @ahmet2mir, I've left a few comments.

gitlab/resource_gitlab_project.go Show resolved Hide resolved
gitlab/resource_gitlab_project.go Show resolved Hide resolved
gitlab/resource_gitlab_project_test.go Show resolved Hide resolved
gitlab/resource_gitlab_project_test.go Show resolved Hide resolved
gitlab/resource_gitlab_project_test.go Show resolved Hide resolved
vendor/github.com/xanzy/go-gitlab/client_options.go Outdated Show resolved Hide resolved
website/docs/r/project.html.markdown Outdated Show resolved Hide resolved
@sfang97
Copy link
Contributor

sfang97 commented Sep 16, 2020

@ahmet2mir This looks good to me, thanks for working on it! Might be worth it to get another set of eyes on this before it goes in. @armsnyder Could you take a look? 😄

@armsnyder armsnyder self-requested a review September 17, 2020 06:10
@armsnyder
Copy link
Collaborator

armsnyder commented Sep 17, 2020

@ahmet2mir Thanks for the detailed response about the template group!

@roidelapluie What do you think about the change to start-gitlab.sh? Ahmet is using the rails console to configure the template project group, which is used in the test, because there's isn't a GitLab API for creating the template project group. (See the explanation here.) Since you know more about the ways tests can be run, do you think it's ok, or do you have any suggestions there?

@sfang97 Sure I'll review.

gitlab/resource_gitlab_project.go Outdated Show resolved Hide resolved
gitlab/resource_gitlab_project.go Outdated Show resolved Hide resolved
gitlab/resource_gitlab_project_test.go Show resolved Hide resolved
gitlab/resource_gitlab_project_test.go Outdated Show resolved Hide resolved
gitlab/resource_gitlab_project_test.go Outdated Show resolved Hide resolved
gitlab/resource_gitlab_project_test.go Outdated Show resolved Hide resolved
@armsnyder
Copy link
Collaborator

Also, just as a reminder @sfang97, we should run this PR from a branch of this repository, so that the EE tests can run, before we merge. I made the mistake of forgetting to do that recently and it was a pain to fix haha.

@ahmet2mir
Copy link
Contributor Author

ahmet2mir commented Sep 17, 2020

@armsnyder should be good now. Let me know if I forget something.
(and if it's ok I will squash commits)

Copy link
Collaborator

@armsnyder armsnyder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! If you don't squash usually we will do it for you when we merge.

@roidelapluie What's your typical workflow for running EE tests? (Also see my earlier question about start-gitlab.sh)

gitlab/resource_gitlab_project_test.go Outdated Show resolved Hide resolved
@nicholasklick
Copy link
Collaborator

@ahmet2mir looks like one conflicting file and then we can get this merged.

@ahmet2mir
Copy link
Contributor Author

@nicholasklick merged master to this branch

* template_name
* template_project_id
* use_custom_template
* group_with_project_templates_id
@ahmet2mir
Copy link
Contributor Author

@armsnyder @sfang97 @roidelapluie I remerged and pushed forced changes due to lot of conflicts with master. Could we merge this or need I rebase everyday :)

@nicholasklick nicholasklick merged commit 05a262e into gitlabhq:master Sep 21, 2020
@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