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 namespcace ID attribute #14483

Conversation

hauleth
Copy link
Contributor

@hauleth hauleth commented May 15, 2017

This commit also introduce id comouted value which is numeric value
used by GitLab to iteract with repository. This should simplify use of
gitlab_project_hook usage and would allow to introduce other resources
as described in #14471

This commit also introduce `id` comouted value which is numeric value
used by GitLab to iteract with repository. This should simplify use of
`gitlab_project_hook` usage and would allow to introduce other resources
as described in hashicorp#14471
@hauleth
Copy link
Contributor Author

hauleth commented May 15, 2017

I am not proficient in Go enough to write tests to cover new functionality though.

"id": {
Type: schema.TypeInt,
Computed: true,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't needed. The reason you cite for adding it is to help work with the gitlab_project_hook resource, but if you look at the acceptance tests for that code you'll see that the id is already exposed and used in its tests: https://github.com/hashicorp/terraform/blob/master/builtin/providers/gitlab/resource_gitlab_project_hook_test.go#L189 Its just an automatically exported attribute that all resources get.

@@ -82,6 +90,7 @@ func resourceGitlabProjectSetToState(d *schema.ResourceData, project *gitlab.Pro
d.Set("snippets_enabled", project.SnippetsEnabled)
d.Set("visibility_level", visibilityLevelToString(project.VisibilityLevel))

d.Set("id", project.ID)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this, it's already done elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@richardc it would be worth documenting then as I cannot see that in gitlab_project Attributes Reference

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, the docs are in the repo too. Probably also worth adding the missing example to gitlab_project_hooks too, so the next user doesn't think that "${gitlab_project.foo.id}" isn't usable already.

richardc added a commit to richardc/terraform that referenced this pull request May 15, 2017
This adds a gitlab_group resource.

This combined with hashicorp#14483 should allow you to create projects in a
group (untested)

The implementation of this is a little ugly as go-gitlab doesn't have a
function already present for EditGroup, so we hack it into being as part
of the implementation of  resourceGitlabGroupUpdate
@richardc
Copy link
Contributor

Cherry-picking this over to my gitlab_group branch (resulting frankenbranch at https://github.com/richardc/terraform/tree/feature/add_gitlab_group_plus_cherry) does result in the ability to create a project in a group with as simply as this:

resource "gitlab_group" "test1" {
  name = "test1"
  path = "test1"
  visibility_level = "public"
}

resource "gitlab_project" "test1" {
  name = "test1"
  namespace_id = "${gitlab_group.test1.id}"
  description = "A test project! Modify 2"
  visibility_level = "public"
  //issues_enabled = false
}

👍 though it will need docs and tests.

@@ -91,6 +95,7 @@ func resourceGitlabProjectCreate(d *schema.ResourceData, meta interface{}) error
client := meta.(*gitlab.Client)
options := &gitlab.CreateProjectOptions{
Name: gitlab.String(d.Get("name").(string)),
NamespaceID: gitlab.Int(d.Get("namespace_id").(int)),
Copy link
Contributor

Choose a reason for hiding this comment

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

One change needed here, this is optional, so it needs to move from here and go with the d.GetOk("") patterned stuff:

if v, ok := d.GetOk("namespace_id"); ok {
   options.NamespaceID = gitlab.Int(v.(int))
}

@richardc
Copy link
Contributor

Spoke too soon, it breaks the case where a namespace_id isn't provided as you're not handling the optional nature correctly. Notes on how to fix it on the line.

@hauleth
Copy link
Contributor Author

hauleth commented May 15, 2017 via email

richardc added a commit to richardc/terraform that referenced this pull request May 19, 2017
This adds a gitlab_group resource.

This combined with hashicorp#14483 will allow you to create projects in a
group.

The implementation of this is a little ugly as go-gitlab doesn't have a
function already present for EditGroup, so we hack it into being as part
of the implementation of resourceGitlabGroupUpdate
richardc added a commit to richardc/terraform that referenced this pull request May 19, 2017
This adds a gitlab_group resource.

This combined with hashicorp#14483 will allow you to create projects in a
group.

The implementation of this is a little ugly as go-gitlab doesn't have a
function already present for EditGroup, so we hack it into being as part
of the implementation of resourceGitlabGroupUpdate
richardc added a commit to richardc/terraform that referenced this pull request May 19, 2017
This adds a gitlab_group resource.

This combined with hashicorp#14483 will allow you to create projects in a
group.
stack72 pushed a commit that referenced this pull request May 24, 2017
* vendor: Update go-gitlab to master@e6c11e

Update go-gitlab to master@e6c11e.  This brings in UpdateGroup in
addition to fuller management of other attributes.

* provider/gitlab:  Add `gitlab_group` resource

This adds a gitlab_group resource.

This combined with #14483 will allow you to create projects in a
group.
@stack72
Copy link
Contributor

stack72 commented May 24, 2017

Hi @hauleth

Thanks for the work here and thanks @richardc for the review. I merged #14490 first and now the tests pass as expected:

make testacc TEST=./builtin/providers/gitlab                                                                                         1 ↵ ✭
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/24 13:03:02 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/gitlab -v  -timeout 120m
=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccGitlabDeployKey_basic
--- PASS: TestAccGitlabDeployKey_basic (36.70s)
=== RUN   TestAccGitlabGroup_basic
--- PASS: TestAccGitlabGroup_basic (21.09s)
=== RUN   TestAccGitlabProjectHook_basic
--- PASS: TestAccGitlabProjectHook_basic (31.13s)
=== RUN   TestAccGitlabProject_basic
--- PASS: TestAccGitlabProject_basic (29.59s)
=== RUN   TestGitlab_validation
--- PASS: TestGitlab_validation (0.00s)
=== RUN   TestGitlab_visbilityHelpers
--- PASS: TestGitlab_visbilityHelpers (0.00s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/gitlab	118.526s

One thing to note - this new param hasn't had any documentation included. Please can you follow up with that?

Thanks

Paul

@stack72 stack72 merged commit 18f2edf into hashicorp:master May 24, 2017
@hauleth hauleth deleted the feat/add-namespace_id-and-project_id-to-gitlab_project-resource branch May 24, 2017 12:06
@ghost
Copy link

ghost commented Apr 12, 2020

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 ghost locked and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants