provider/gitlab: add a gitlab provider #13898
Conversation
Add go-gitlab as a dependency for the gitlab provider
Here we add a basic provider with a single resource type.
It's copied heavily from the `github` provider and `github_repository`
resource, as there is some overlap in those types/apis.
~~~
resource "gitlab_project" "test1" {
name = "test1"
visibility_level = "public"
}
~~~
We implement in terms of the
[go-gitlab](https://github.com/xanzy/go-gitlab) library, which provides
a wrapping of the [gitlab api](https://docs.gitlab.com/ee/api/)
We have been a little selective in the properties we surface for the
project resource, as not all properties are very instructive.
Notable is the removal of the `public` bool as the `visibility_level`
will take precedent if both are supplied which leads to confusing
interactions if they disagree.
|
This was previously proposed in PR #9591 which I'd stalled out on. |
|
Hi @richardc What version of Gitlab do we need to be able to run the tests for this? Will the community edition suffice? Thanks Paul |
|
The community edition is what I developed against using a local gitlab-ce docker image. I built off latest about a week ago so that gave me a snapshot that reports itself as version 9.0.5. I've also run the tests using the free tier of hosting at gitlab.com, which is currently reporting being version 9.1.0. |
| "base_url": &schema.Schema{ | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| DefaultFunc: schema.EnvDefaultFunc("GITLAB_BASE_URL", ""), |
There was a problem hiding this comment.
should we not have a default here? In github provider, we default to public github
There was a problem hiding this comment.
Can do, and it will simplify the code flow in this path: https://github.com/richardc/terraform/blob/feature/gitlab_provider/builtin/providers/gitlab/config.go#L16
| options.Description = gitlab.String(v.(string)) | ||
| } | ||
|
|
||
| if v, ok := d.GetOk("issues_enabled"); ok { |
There was a problem hiding this comment.
as there is a default for bools like this, we can add these straight to the CreateProjectOptions struct
There is a known bug with d.GetOk and bool values like false and ints like 0, so defaults mean it will always be set and we don't need to use d.GetOk :)
| return err | ||
| } | ||
|
|
||
| d.SetId(fmt.Sprintf("%d", project.ID)) |
There was a problem hiding this comment.
why are you needing to use fmt.Sprintf? d.SetId takes a string and you are formatting as as int
There was a problem hiding this comment.
project.ID is an int.
| } | ||
| } | ||
|
|
||
| func resourceGitlabProjectUpdateFromAPI(d *schema.ResourceData, project *gitlab.Project) { |
There was a problem hiding this comment.
I would rename this func to something like resourceGitlabProjectSetToState
It doesn't interact with the API at all and just sets to state
There was a problem hiding this comment.
Much better name 👍
|
|
||
| d.SetId(fmt.Sprintf("%d", project.ID)) | ||
|
|
||
| resourceGitlabProjectUpdateFromAPI(d, project) |
There was a problem hiding this comment.
in this case, Create should call the Read func - that way we know that the project has been created
Right now, we are just setting the values the user specified back to state
Therefore, this should be return resourceGitlabProjectRead(d, meta)
There was a problem hiding this comment.
Not according to my reading of the gitlab code. POST /projects returns a json document that is the state of the project once it has been created. It is the same project info, so by making this tail call Read you're just creating extra unecessary api hits.
> POST /projects {:spec of project}
< project state
Then you propose an extra
> GET /projects/:id
< project state
| return err | ||
| } | ||
|
|
||
| resourceGitlabProjectUpdateFromAPI(d, project) |
There was a problem hiding this comment.
Updates should also call Read to make sure that the API has persisted the changes
so again, return resourceGitlabProjectRead(d, meta)
There was a problem hiding this comment.
It's already doing that. You're adding an extra api round trip, which isn't needed.
|
|
||
| func resourceGitlabProjectDelete(d *schema.ResourceData, meta interface{}) error { | ||
| client := meta.(*gitlab.Client) | ||
| log.Printf("[DEBUG] update gitlab project %s", d.Id()) |
|
Hi @richardc I am going to go ahead and make these small changes - you have worked really hard on this and it would be unfair for me to ask you to make these small changes :) I will ping you when i merge it Thanks Paul |
|
Hi @richardc I made the changes here - they took me like 10 minutes to add :) Your work is very much appreciated! It will be great to get this as part of Terraform 0.9.5 :) Thanks Paul |
|
Uh, I disagree with the restructuring to tail-call Read; it's an extra set of api calls per operation that aren't really needed, but if it's house style then go for it. |
|
@ricardclau I am looking at your response now and will update it - as i said above, you had this all working as expected, there were just a few tiny nits - but nothing crazy I'm looking at the docs for the Create and the Update now to make sure all works as expected P. |
|
OK great. I'm excited to see this merged |
|
works a dream sir :) just took it for a test spin |
|
@richardc great work 👍 |
|
Will it be possible to create project in group, not in user scope? |
|
I'll caveat my response with a mild disclaimer: I've not looked at doing this in depth, as it wasn't my initial use case. The API wrapped defaults to operating on the currently authenticated user, as determined by the API key configured for the provider. If it's possible to issue an API key to a group then it should just shake out already, otherwise extra work will be needed to figure out how the API models this, and to make the code changes. If the changes are needed to make it work I'd consider it new functionality, so please raise a ticket and mention me in it. https://github.com/xanzy/go-gitlab/blob/v0.5.1/projects.go#L360 https://docs.gitlab.com/ce/api/projects.html#create-project |
|
I believe that |
|
What is more it would be unbelievably more useful if we would have possibility to:
I know that this is a little too much to ask someone to do work like this, but unfortunately I am not gophery enough to create such resources. |
|
I've got you covered for setting project hooks already #14155 I agree the rest could be useful, but you're commenting on a merged pull-request, which is why I suggest you file enhancement requests for anything else. |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
This adds a gitlab provider and
gitlab_projectresource. It's copied heavily from thegithubprovider andgithub_repositoryresource, as there is some overlap in those types/apis.Sample usage:
Its implemented in terms of the go-gitlab library, which provides a wrapping of the gitlab api
We have been a little selective in the properties we surface for the project resource, as not all properties are very instructive.
Notable is the removal of the
publicbool as thevisibility_levelwill take precedent if both are supplied which leads to confusing interactions if they disagree.