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

Fix and rework gitlab_project_variable #4038

Conversation

markuman
Copy link
Member

@markuman markuman commented Jan 13, 2022

SUMMARY

Closes: #1952
Closes: #4074

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

gitlab_project_variable

ADDITIONAL INFORMATION

First tests looks promising.

    - name: previous way. still works
      community.general.gitlab_project_variable:
        api_url: https://gitlab.com
        api_token: "{{ some_token }}"
        project: markuman/dotfiles
        purge: false
        vars:
          ACCESS_KEY_ID: abc123
          SECRET_ACCESS_KEY:
            value: 3214cbad
            masked: true
            protected: true
            variable_type: env_var
            environment_scope: '*'

    - name: new way, works also
      community.general.gitlab_project_variable:
        api_url: https://gitlab.com
        api_token: "{{ some_token }}"
        project: markuman/dotfiles
        purge: false
        variables:
          - name: ACCESS_KEY_ID
            value: abc123
          - name: SECRET_ACCESS_KEY
            value: 3214cbad
            masked: true
            protected: true
            variable_type: env_var
            environment_scope: '*'

What's left is

  • append integration test
  • return_value[untouched] is not handled
  • changelog fragment

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug gitlab module module plugins plugin (any type) source_control labels Jan 13, 2022
@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 13, 2022
@markuman markuman marked this pull request as draft January 13, 2022 19:10
@ansibullbot ansibullbot added the WIP Work in progress label Jan 13, 2022
@markuman markuman marked this pull request as ready for review January 13, 2022 21:46
@markuman
Copy link
Member Author

Integration test pass for me

PLAY RECAP *********************************************************************
testhost                   : ok=70   changed=27   unreachable=0    failed=0    skipped=0    rescued=0    ignored=0  

@ansibullbot ansibullbot added integration tests/integration tests tests and removed WIP Work in progress labels Jan 13, 2022
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! Some first comments:

markuman and others added 2 commits January 27, 2022 08:56
Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
@ansibullbot ansibullbot removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 27, 2022
@markuman
Copy link
Member Author

All integration tests still pass here

testhost                   : ok=70   changed=27   unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

@ansibullbot ansibullbot added needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 28, 2022
@markuman
Copy link
Member Author

Still no failures

testhost                   : ok=70   changed=26   unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

FYI, the changed count flipps between 26 and 27 because of the first task, which purges all variables from the test project. If it is already empty, it's one change less

@markuman
Copy link
Member Author

hm codespace put the changes somewhere else ...

…compare, because values always exists (prebuild)
@felixfontein felixfontein merged commit 33a65ae into ansible-collections:main Jan 31, 2022
@patchback
Copy link

patchback bot commented Jan 31, 2022

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/33a65ae20f53ead447ca08d1c0c7244c067ec445/pr-4038

Backported as #4133

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 31, 2022
* rework-and-fix

* fix delete bug and change report

* delete the requested variables based on env scope

* fix absent logic when not purge: remove what is requested

* change code to current behaviour

* complete implementation

* fix delete

* restore origin return structure

* reorder

* add test for origin bug

* add changelog fragment

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* remove yaml

* apply suggestions

* readd accidental removed line

* improve the truth of return value 'project_variable' in check mode

* fix pep8, over-indented

* fix typos and add subelement options

* Update changelogs/fragments/4038-fix-and-rework-gitlb-project-variable.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelogs/fragments/4038-fix-and-rework-gitlb-project-variable.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* remove diff feature

* resolve all recommentdations

* resolve change requests, improve doc and remove default value before compare, because values always exists (prebuild)

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 33a65ae)
@felixfontein
Copy link
Collaborator

@markuman thanks a lot for implementing this!
@sguarin thanks for reviewing!

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jan 31, 2022
felixfontein pushed a commit that referenced this pull request Jan 31, 2022
* rework-and-fix

* fix delete bug and change report

* delete the requested variables based on env scope

* fix absent logic when not purge: remove what is requested

* change code to current behaviour

* complete implementation

* fix delete

* restore origin return structure

* reorder

* add test for origin bug

* add changelog fragment

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* remove yaml

* apply suggestions

* readd accidental removed line

* improve the truth of return value 'project_variable' in check mode

* fix pep8, over-indented

* fix typos and add subelement options

* Update changelogs/fragments/4038-fix-and-rework-gitlb-project-variable.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelogs/fragments/4038-fix-and-rework-gitlb-project-variable.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/source_control/gitlab/gitlab_project_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* remove diff feature

* resolve all recommentdations

* resolve change requests, improve doc and remove default value before compare, because values always exists (prebuild)

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 33a65ae)

Co-authored-by: Markus Bergholz <git@osuv.de>
sguarin added a commit to sguarin/community.general that referenced this pull request Feb 2, 2022
felixfontein added a commit that referenced this pull request Feb 18, 2022
* Rework of gitlab_project_variable over gitlab_group_variable

* Linting and removed unused example

* Fix test 2

* Sync from review of gitlab_project_variable #4038

* Linting, default protected True, value optional

* Next version is 4.5.0

* Roll back protected default true, and value not required

* Apply suggestions from code review

Missing check_mode

Co-authored-by: Markus Bergholz <git@osuv.de>

* Fix one unit test, comment test that requires premium gitlab

* Add changelog

* Update plugins/modules/source_control/gitlab/gitlab_group_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Added conditional gitlab_premium_tests variable when required

* Allow delete without value

* Fix variable name

* Linting

* Value should not be required in doc

* Linting missing new-line

* Update changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml

Co-authored-by: Markus Bergholz <git@osuv.de>

Co-authored-by: Markus Bergholz <git@osuv.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
patchback bot pushed a commit that referenced this pull request Feb 18, 2022
* Rework of gitlab_project_variable over gitlab_group_variable

* Linting and removed unused example

* Fix test 2

* Sync from review of gitlab_project_variable #4038

* Linting, default protected True, value optional

* Next version is 4.5.0

* Roll back protected default true, and value not required

* Apply suggestions from code review

Missing check_mode

Co-authored-by: Markus Bergholz <git@osuv.de>

* Fix one unit test, comment test that requires premium gitlab

* Add changelog

* Update plugins/modules/source_control/gitlab/gitlab_group_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Added conditional gitlab_premium_tests variable when required

* Allow delete without value

* Fix variable name

* Linting

* Value should not be required in doc

* Linting missing new-line

* Update changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml

Co-authored-by: Markus Bergholz <git@osuv.de>

Co-authored-by: Markus Bergholz <git@osuv.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 44f9bf5)
felixfontein pushed a commit that referenced this pull request Feb 18, 2022
…4226)

* Rework of gitlab_project_variable over gitlab_group_variable

* Linting and removed unused example

* Fix test 2

* Sync from review of gitlab_project_variable #4038

* Linting, default protected True, value optional

* Next version is 4.5.0

* Roll back protected default true, and value not required

* Apply suggestions from code review

Missing check_mode

Co-authored-by: Markus Bergholz <git@osuv.de>

* Fix one unit test, comment test that requires premium gitlab

* Add changelog

* Update plugins/modules/source_control/gitlab/gitlab_group_variable.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Added conditional gitlab_premium_tests variable when required

* Allow delete without value

* Fix variable name

* Linting

* Value should not be required in doc

* Linting missing new-line

* Update changelogs/fragments/4086-rework_of_gitlab_proyect_variable_over_gitlab_group_variable.yml

Co-authored-by: Markus Bergholz <git@osuv.de>

Co-authored-by: Markus Bergholz <git@osuv.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 44f9bf5)

Co-authored-by: Sebastian Guarino <sebastian.guarino@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug gitlab has_issue integration tests/integration module module plugins plugin (any type) source_control tests tests
Projects
None yet
4 participants