Extend API: Group Secret Variable
What does this MR do?
This MR adds API support for Group-level Secret Variable. Group-level Secret Variable was introduced at %9.4.
Are there points in the code the reviewer needs to double check?
This is the first API for Group-level settings of CI.
Why was this MR needed?
We should support API when introduced a new feature.
Screenshots (if relevant)
N/A
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated -
API support added - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the merge request performance guides -
Conform by the style guides -
Branch has no merge conflicts with master
(if it does - rebase it please) -
Squashed related commits together
What are the relevant issue numbers?
Closes #34519 (closed)
Merge request reports
Activity
marked the checklist item Changelog entry added, if necessary as completed
marked the checklist item Conform by the merge request performance guides as completed
marked the checklist item Conform by the style guides as completed
marked the checklist item Squashed related commits together as completed
marked the checklist item Documentation created/updated as completed
added 77 commits
-
3c99dc61...045e4de4 - 74 commits from branch
master
- b5399517 - Ini
- 280cfc61 - Add changelog
- 862e2c80 - Document update
Toggle commit list-
3c99dc61...045e4de4 - 74 commits from branch
@ayufan Could you review this? Or if you're busy, could you ask someone to review this?
Pipeline is green! Everything is ready! Basically I copied logic/code from project-level variables.
Thank you!
assigned to @ayufan
@ayufan We can't change approvers. It's known issue. https://gitlab.slack.com/archives/C02PF508L/p1501577697310720.
mentioned in commit 8ffd40ce
mentioned in issue #35825 (closed)
mentioned in merge request !13227 (merged)
This requires an EE version, discovered in https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2589 from job https://gitlab.com/gitlab-org/gitlab-ee/-/jobs/28149498 fixed at https://gitlab.com/gitlab-org/gitlab-ee/commit/f9cc6aadf36a282dfb328d1e2c60a4efbbf91d9b
Oops, this is clearly why
respond_to?
could be bad. It doesn't detect typo. (same withdefined?
) https://gitlab.com/gitlab-org/gitlab-ee/commit/57fbfeb697e0e2b545c096c16b13fa5ba7bfe68cTo properly fix it, we should maybe define a unified method. Could be called like
owner
, if that's not confusing with user.@godfat Thank you for fixing this conflict.
I think we should fix with
@variable.respond_to?(:environment_scope)
. So it'd look likeclass Variable < Grape::Entity expose :key, :value expose :protected?, as: :protected # EE expose :environment_scope, if: ->(variable, options) { variable.project.feature_available?(:variable_environment_scope) if @variable.respond_to?(:environment_scope) } end
The reason is
- This entity is used by
Ci::Variable
andCi::GroupVariable
- Only Project variable support environment_scope.
- Group variable doesn't support
environment_scope
yet. https://gitlab.com/gitlab-org/gitlab-ee/issues/2874 - We've distinguished ProjectVariable and GroupVariable by
respond_to?(:environment_scope)
so far. e.g. https://gitlab.com/gitlab-org/gitlab-ee/blob/master/app/views/ci/variables/_table.html.haml#L12
I agree with that we need a unified method. Feel free to create an ~"technical debt" issue. Maybe we should extend presenter?
- This entity is used by
@dosuken123 You're right, pushed at https://gitlab.com/gitlab-org/gitlab-ee/commit/570813d605a15f85064d2a636d67f2a3661ee774 Thanks. Hope this is the last commit.
@godfat thank you!
Tbh, l'm worried about when you're sleeping rather than those implementations. Please take a rest :D
mentioned in issue #36543 (closed)