Skip to content

fix: variables with same name but different scope should be deletable#962

Merged
TimKnight-DWP merged 36 commits intogitlabform:v5from
amimas:issue-533
Sep 17, 2025
Merged

fix: variables with same name but different scope should be deletable#962
TimKnight-DWP merged 36 commits intogitlabform:v5from
amimas:issue-533

Conversation

@amimas
Copy link
Copy Markdown
Collaborator

@amimas amimas commented Feb 17, 2025

This PR fixes the bug reported in #533. I think this is an important bug that unfortunately hadn't been fixed yet.

As per the referenced issue, if there are multiple CI CD variables with same key/name but scoped to different "environment", gitlabform does not make the API call correctly. As a result, the variable cannot be deleted and an error is returned.

There's a lot of changes in this PR, but here's an overview of the changes for better understanding:

  • Adds more test cases that previously weren't available: This helped to reproduce the bug and also added more test scenarios.
  • Migrate the project and group variables processor to use python-gitlab: This would be required later anyways and by doing it now we avoid modifying shared code (MultipleEntitiesProcessor), which can unintentionally affect other processors and introduce other bugs in case of lack of tests.
  • Cleanup codes as a result of migration to python-gitlab

fixes: #533

@amimas amimas temporarily deployed to Integrate Pull Request February 17, 2025 02:11 — with GitHub Actions Inactive
@amimas amimas temporarily deployed to Integrate Pull Request February 17, 2025 02:11 — with GitHub Actions Inactive
@amimas amimas requested review from TimKnight-DWP, gdubicki and jimisola and removed request for TimKnight-DWP, gdubicki and jimisola February 17, 2025 02:11
@amimas amimas temporarily deployed to Integrate Pull Request February 17, 2025 02:13 — with GitHub Actions Inactive
@amimas amimas temporarily deployed to Integrate Pull Request February 17, 2025 02:13 — with GitHub Actions Inactive
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 17, 2025

Codecov Report

❌ Patch coverage is 81.56028% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.37%. Comparing base (ad769af) to head (333ec09).
⚠️ Report is 1 commits behind head on v5.

Files with missing lines Patch % Lines
.../processors/project/project_variables_processor.py 53.57% 26 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##               v5     #962      +/-   ##
==========================================
+ Coverage   83.28%   88.37%   +5.09%     
==========================================
  Files          73       73              
  Lines        3565     3640      +75     
==========================================
+ Hits         2969     3217     +248     
+ Misses        596      423     -173     
Flag Coverage Δ
integration 86.34% <81.56%> (+7.77%) ⬆️
unittests 38.84% <27.65%> (-0.74%) ⬇️
Files with missing lines Coverage Δ
gitlabform/gitlab/__init__.py 100.00% <ø> (ø)
gitlabform/gitlab/projects.py 50.00% <ø> (+0.79%) ⬆️
...form/processors/group/group_variables_processor.py 100.00% <100.00%> (ø)
gitlabform/processors/project/__init__.py 100.00% <100.00%> (ø)
gitlabform/processors/util/variables_processor.py 100.00% <100.00%> (ø)
.../processors/project/project_variables_processor.py 53.57% <53.57%> (ø)

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gdubicki gdubicki temporarily deployed to Integrate Pull Request February 17, 2025 11:06 — with GitHub Actions Inactive
@gdubicki gdubicki temporarily deployed to Integrate Pull Request February 17, 2025 11:06 — with GitHub Actions Inactive
@amimas amimas had a problem deploying to Integrate Pull Request February 17, 2025 15:32 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request February 17, 2025 15:32 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request February 17, 2025 15:39 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request February 17, 2025 15:39 — with GitHub Actions Failure
@amimas amimas temporarily deployed to Integrate Pull Request February 17, 2025 15:43 — with GitHub Actions Inactive
@amimas amimas temporarily deployed to Integrate Pull Request February 17, 2025 15:43 — with GitHub Actions Inactive
@amimas amimas had a problem deploying to Integrate Pull Request February 18, 2025 15:23 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request February 18, 2025 15:23 — with GitHub Actions Failure
@amimas amimas temporarily deployed to Integrate Pull Request February 18, 2025 15:23 — with GitHub Actions Inactive
@amimas amimas temporarily deployed to Integrate Pull Request February 18, 2025 15:23 — with GitHub Actions Inactive
Changed test structure to follow rest of the repo convention and it also makes it easy to follow the test cases. Caught a couple of bugs that have been fixed as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Variable deletion is unsuccessful due to environment scope

4 participants