xanzy / go-gitlab

GitLab Go SDK
Apache License 2.0
2.32k stars 922 forks source link

Check for Environment Scope in ReadVariable #746

Open oesah opened 4 years ago

oesah commented 4 years ago

I am coming here from https://github.com/terraform-providers/terraform-provider-gitlab.

Problem We are using go-gitlab to create resources using terraform. We face an issue, however, where project or group variables do not consider the scope. So if you have 2 variables with the same name but different scopes, it will cause terraform to always try to update them. See discussion here: https://github.com/terraform-providers/terraform-provider-gitlab/issues/213

I think the issue lie here: https://github.com/xanzy/go-gitlab/blob/master/group_variables.go#L84 https://github.com/xanzy/go-gitlab/blob/master/project_variables.go#L86

Possible Solutions Currently, the Gitlab API does not allow to filter them. You can follow the issue here: https://gitlab.com/gitlab-org/gitlab/issues/20661

Once that bug is fixed, we can enhance the above mentioned functions to incorporate the filtering solution.

abelmokadem commented 4 years ago

As an intermediate solution, can't you just query the list of all variables and then pick the variable from there?

/api/v4/projects/<project_id>/variables

I know that this is not a "nice" solution, but it would solve the issue we also have in the terraform project.

remil1000 commented 3 years ago

There is an update coming (probably end of July 2020) addressing this issue with a feature flag to be enabled _ci_variables_api_filter_environmentscope

edit: it's not a request, only a heads up; I'll try to take a look and send a PR once it's released

enkov commented 3 years ago

Any news? Gitlab fix the issue and released it.

remil1000 commented 3 years ago

made an attempt here https://github.com/remil1000/go-gitlab/tree/9912-project-variable-environment-scope used and tested in https://github.com/remil1000/terraform-provider-gitlab/tree/9912-project-variable-environment-scope

don't forget to enable the Gitlab feature through Gitlab console sudo gitlab-rails console and Feature.enable(:ci_variables_api_filter_environment_scope)

woz5999 commented 3 years ago

@remil1000 do you plan to submit a PR with your changes?

martinhanzik commented 3 years ago

@remil1000 It would be really nice to get this into a PR so we can finally get the Terraform resource working properly, do you plan to submit this? If not, I will reimplement it and submit it myself, but I don't see the point in doing this, if you've already done it.

remil1000 commented 3 years ago

I should make a PR, but in the mean time I discovered there may be a simpler and prettier way of doing this it seems the NewRequest function argument includes an opt interface{} to construct query string from a struct of options for each CRUD method; an example of such struct for the Update/PUT method is

type UpdateProjectVariableOptions struct {
  Value            *string            `url:"value,omitempty" json:"value,omitempty"`
  VariableType     *VariableTypeValue `url:"variable_type,omitempty" json:"variable_type,omitempty"`
  Protected        *bool              `url:"protected,omitempty" json:"protected,omitempty"`
  Masked           *bool              `url:"masked,omitempty" json:"masked,omitempty"`
  EnvironmentScope *string            `url:"environment_scope,omitempty" json:"environment_scope,omitempty"`
}

so probably adding an url:"filter[environment_scope],omitempty" would do the trick I still need to check on that

if you have any suggestion let me know

martinhanzik commented 3 years ago

@remil1000 Any progress on this?

phikai commented 1 month ago

Looks like filter was officially added to the GitLab API in 16.9, so potentially this is more possible now if anyone is interested. https://docs.gitlab.com/ee/api/group_level_variables.html#the-filter-parameter - There's some ask for it on the GitLab CLI as well: https://gitlab.com/gitlab-org/cli/-/issues/7436

Farfaday commented 1 week ago

I guess fixing this would then allow fixing https://github.com/external-secrets/external-secrets/issues/3379.