Add instance cluster API support #893
Conversation
Instance cluster API support was added to GitLab in 13.2, so add instance cluster support to TF provider
There was a problem hiding this comment.
This looks very good @sfang97! Thanks!
Do have a few change requests for the naming and struct usage, but only small things. Code looks good 👍
instance_clusters.go
Outdated
| // | ||
| // GitLab API docs: | ||
| // https://docs.gitlab.com/ee/api/instance_clusters.html#add-existing-cluster-to-instance | ||
| type AddInstanceClusterOptions struct { |
There was a problem hiding this comment.
| type AddInstanceClusterOptions struct { | |
| type AddClusterOptions struct { |
There was a problem hiding this comment.
AddClusterOptions is already declared in https://github.com/xanzy/go-gitlab/blob/d3946d4f8c62bfae945062be289f13ea5e6ecfe4/project_clusters.go#L129
There was a problem hiding this comment.
Same for this one, if the struct already exists, why not reuse it here versus declaring the same struct twice?
instance_clusters.go
Outdated
| } | ||
|
|
||
| // PlatformKubernetes represents a GitLab Instance Cluster PlatformKubernetes. | ||
| type InstanceClusterPlatformKubernetes struct { |
There was a problem hiding this comment.
| type InstanceClusterPlatformKubernetes struct { | |
| type PlatformKubernetes struct { |
There was a problem hiding this comment.
Hi @svanharmelen , thanks for the suggestion. I tried applying it but the struct PlatformKubernetes is already declared in https://github.com/xanzy/go-gitlab/blob/master/project_clusters.go:56:6: PlatformKubernetes redeclared in this block
There was a problem hiding this comment.
So why not just omit the struct here and use the existing one?
There was a problem hiding this comment.
Gotcha I see what you mean, pushed these changes!
instance_clusters.go
Outdated
| // | ||
| // GitLab API docs: | ||
| // https://docs.gitlab.com/ee/api/instance_clusters.html#edit-instance-cluster | ||
| type EditInstanceClusterOptions struct { |
There was a problem hiding this comment.
| type EditInstanceClusterOptions struct { | |
| type EditClusterOptions struct { |
There was a problem hiding this comment.
Same thing here, EditClusterOptions is already declared in https://github.com/xanzy/go-gitlab/blob/d3946d4f8c62bfae945062be289f13ea5e6ecfe4/project_clusters.go#L177
|
@svanharmelen Thanks for the review! I applied the non-conflicting suggestions. As for the others, the compiler complains that the new name is |
instance_clusters.go
Outdated
| } | ||
|
|
||
| // PlatformKubernetes represents a GitLab Instance Cluster PlatformKubernetes. | ||
| type InstanceClusterPlatformKubernetes struct { |
There was a problem hiding this comment.
So why not just omit the struct here and use the existing one?
instance_clusters.go
Outdated
| // | ||
| // GitLab API docs: | ||
| // https://docs.gitlab.com/ee/api/instance_clusters.html#add-existing-cluster-to-instance | ||
| type AddInstanceClusterOptions struct { |
There was a problem hiding this comment.
Same for this one, if the struct already exists, why not reuse it here versus declaring the same struct twice?
|
@svanharmelen Thanks for the review, I removed the unused structs. Ready to merge? 🚀 |
Instance cluster API support was added to GitLab in 13.2, so this PR adds support for it. This is similar to the project/group cluster integration, but unlike the project/group implementation, there is no instance ID value since any instance cluster applies to the entire GitLab instance.
Instance cluster API documentation