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

feat: add allowed_to_create support for tag protection #551

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

amimas
Copy link
Collaborator

@amimas amimas commented Jun 4, 2023

Currently gitlabform does not support configuring tag protection using specific user(s) or group(s) as "Allowed to create". Only basic role can be set, such as: developer, maintainer, etc. Selecting individual user or group is a feature of Premium or Ultimate license.

This change adds the ability to configure tag protection to be set to individual users or groups. They can be set as a user/group ID or their names. For example with a config like below, a tag named special-tag-blah can only be created by either user Jane.Doe, or user whose ID is 123 or, users who are member of the group foo-bar.

projects_and_groups:
  group_1/project_1:
    tags:
      "v*":
        protected: true
        create_access_level: developer
      "special-tag-*":
        protected: true
        allowed_to_create:
          - user: Jane.Doe
          - user_id: 123
          - group: foo-bar
      "some-old-tag":
        protected: false

To Dos:

  • Implement feature
  • Add acceptance test
  • Update documentation

closes #505

@amimas amimas temporarily deployed to Integrate Pull Request June 4, 2023 17:49 — with GitHub Actions Inactive
@amimas amimas marked this pull request as draft June 4, 2023 17:50
@codecov
Copy link

codecov bot commented Jun 4, 2023

Codecov Report

Merging #551 (70a6e22) into main (9b20242) will increase coverage by 0.78%.
The diff coverage is 100.00%.

❗ Current head 70a6e22 differs from pull request most recent head 0da9a15. Consider uploading reports for the commit 0da9a15 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #551      +/-   ##
==========================================
+ Coverage   82.18%   82.96%   +0.78%     
==========================================
  Files          70       70              
  Lines        2672     2689      +17     
==========================================
+ Hits         2196     2231      +35     
+ Misses        476      458      -18     
Impacted Files Coverage Δ
gitlabform/gitlab/tags.py 84.21% <100.00%> (+4.21%) ⬆️
gitlabform/processors/project/tags_processor.py 92.59% <100.00%> (+10.44%) ⬆️

... and 4 files with indirect coverage changes

@amimas
Copy link
Collaborator Author

amimas commented Jun 4, 2023

@gdubicki - My first time trying to contributing a feature in gitlabform 😊 . I'm not sure how to handle the acceptance test since this is a premium feature. I've worked on it outside of daily work. So, I don't have access to a premium licensed repo. Do you have any suggestions?

@gdubicki
Copy link
Member

gdubicki commented Jun 4, 2023

Hi @amimas! Thank you for working on this. :)

You can write the acceptance test and run it on GitLab with premium license here, in GitHub actions. It requires an approval for a "deployment" as this is a way to secure the license from being, f.e. send over to some malicious user that would create a PR that would do that. As of now the permissions to approve such "deployment" have: you, @jimisola and me. :)

@amimas
Copy link
Collaborator Author

amimas commented Jun 5, 2023

Sounds good @gdubicki . I'll try out that approach, when I can look into this again.

@amimas amimas had a problem deploying to Integrate Pull Request June 6, 2023 13:09 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 6, 2023 15:27 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 7, 2023 01:11 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 7, 2023 02:32 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 7, 2023 03:32 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 7, 2023 13:23 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 7, 2023 13:47 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 8, 2023 00:28 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 8, 2023 11:58 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 8, 2023 12:26 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 8, 2023 12:43 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 9, 2023 23:55 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 10, 2023 15:03 — with GitHub Actions Failure
@amimas amimas had a problem deploying to Integrate Pull Request June 10, 2023 20:52 — with GitHub Actions Failure
@amimas
Copy link
Collaborator Author

amimas commented Jun 11, 2023

Hi @gdubicki - I'm having some trouble with this and wondering if you have time to help. I don't usually work with python. So, there could be improvements. But, right now I'm getting some unexpected failure from the acceptance test.

Error: Error occurred while processing project gitlabform_group_mandarin/gitlabform_project_venue, exception:

Request url='http://localhost/api/v4/projects/gitlabform_group_mandarin%2Fgitlabform_project_venue/protected_tags', method=POST, data='{"allowed_to_create": [{"access_level": 0}, {"user_id": 44}, {"user_id": 45}], "name": "gitlabform_tag_attendee"}' failed - expected code(s) [201], got code 400 & body: '{"error":"allowed_to_create is invalid"}'

It's been time consuming to test out the change since I don't have a local gitlab instance with premium license. I could only try every change/commit in this PR. But, looking at the above error, it doesn't really make sense to me.

According to protected tag api doc, the data looks alright to me. Not sure why the error is saying allowed_to_create is invalid.

I did try similar data in a project in a gitlab instance I have access to. It worked fine in there.

@jimisola
Copy link
Collaborator

Very unfamiliar with this specific API but can you really have multiple user_id with a single allowed_to_create?

Array of access levels allowed to create tags, with each described by a hash of the form {user_id: integer}, {group_id: integer}, or {access_level: integer}.

It does not say that it can be multiple user_id or?

@amimas
Copy link
Collaborator Author

amimas commented Jun 11, 2023

Hi @jimisola . yes you can have multiple user id or group id. If you look at the error message snippet I posted above, you'll see the api call that includes the data.

I tried the same data in my work gitlab project and the protected tag was created as expected. Now that I think about it, initially it failed with a different error message. I had to add the Content-Type: application/json header to the api call. Not sure if that's one of the issue here but the error message doesn't match in that case.

@amimas amimas requested a deployment to Integrate Pull Request June 24, 2023 08:09 — with GitHub Actions Waiting
@gdubicki gdubicki had a problem deploying to Integrate Pull Request July 12, 2023 13:57 — with GitHub Actions Failure
@gdubicki gdubicki had a problem deploying to Integrate Pull Request July 12, 2023 15:31 — with GitHub Actions Failure
@gdubicki gdubicki had a problem deploying to Integrate Pull Request July 14, 2023 09:11 — with GitHub Actions Failure
@gdubicki gdubicki had a problem deploying to Integrate Pull Request July 14, 2023 09:17 — with GitHub Actions Failure
@gdubicki gdubicki had a problem deploying to Integrate Pull Request July 14, 2023 21:56 — with GitHub Actions Failure
@gdubicki
Copy link
Member

Hey @amimas, I think that there an actual problem with the code that makes the tests fail here.

@amimas
Copy link
Collaborator Author

amimas commented Jul 15, 2023

Hey @amimas, I think that there an actual problem with the code that makes the tests fail here.

The error is following as far as I can tell:

gitlabform.gitlab.core.UnexpectedResponseException: Request url='http://localhost/api/v4/projects/gitlabform_group_turbofan%2Fgitlabform_project_cringing/protected_tags', method=POST, data='{"allowed_to_create": [{"access_level": 30}, {"user_id": 55}], "name": "gitlabform_tag_chemist"}' failed - expected code(s) [201], got code 400 & body: '{"error":"allowed_to_create is invalid"}'

Just not sure why the API call is responding with allowed_to_create is invalid. I'll look into it more. Not having EE license to debug this locally is a major issue.

@amimas amimas temporarily deployed to Integrate Pull Request July 19, 2023 01:54 — with GitHub Actions Inactive
@amimas
Copy link
Collaborator Author

amimas commented Jul 19, 2023

@gdubicki - I signed up for a gitlab trial. Got a license for 30 days and that helped debug the issues locally. After a lot of trial and error, I was able to get this to work.

tests/acceptance/premium/test_tags.py .                                                                                                                 [100%]


================================================================ 1 passed =================================================================

Originally the config was being passed to _make_requests_to_api function as dict. That doesn't work for tag protection API unless we set Content-Type: application/json header. After reviewing the branch protection logic, I switched to sending the config as json instead and that seemed to help (although needed to fix test logics).

There is still one issues remaining. I'm able to get the test pass intermittently. Most of the time it fails with this error Create access levels user is not a member of the project:

gitlabform.gitlab.core.UnexpectedResponseException: Request url='http://localhost/api/v4/projects/gitlabform_group_passage%2Fgitlabform_project_arrive/protected_tags?name=gitlabform_tag_filtrate', method=POST, json='{"allowed_to_create": [{"access_level": 0}, {"user_id": 194}, {"user_id": 195}]}' failed - expected code(s) [201], got code 422 & body: '{"message":["Create access levels user is not a member of the project"]}'

DEBUG    root:__init__.py:486 * (1/1) FINISHED Processing project: gitlabform_group_passage/gitlabform_project_arrive
-------------------------------------------------------------------- Captured log teardown --------------------------------------------------------------------
DEBUG    urllib3.connectionpool:connectionpool.py:473 http://localhost:80 "DELETE /api/v4/users/194 HTTP/1.1" 204 0
DEBUG    urllib3.connectionpool:connectionpool.py:473 http://localhost:80 "DELETE /api/v4/users/195 HTTP/1.1" 204 0
DEBUG    urllib3.connectionpool:connectionpool.py:473 http://localhost:80 "DELETE /api/v4/projects/82 HTTP/1.1" 202 26
DEBUG    urllib3.connectionpool:connectionpool.py:473 http://localhost:80 "DELETE /api/v4/groups/gitlabform_group_passage HTTP/1.1" 202 26
=================================================================== short test summary info ===================================================================
FAILED tests/acceptance/premium/test_tags.py::TestTags::test__allowed_to_create_by_user_only - SystemExit: 2
================================================================ 1 failed =================================================================

Not sure why running into the above issue. Unless I missed it, the tests are written similar to branch protection test (tests/acceptance/premium/test_branches.py). Also, the error doesn't come up all the time.

@amimas amimas requested a review from gdubicki July 19, 2023 02:10
@amimas
Copy link
Collaborator Author

amimas commented Jul 19, 2023

There is still one issues remaining. I'm able to get the test pass intermittently. Most of the time it fails with this error Create access levels user is not a member of the project:

I just realized that I run into similar issue locally when I run the tests/acceptance/premium/test_branches.py tests.

gitlabform.gitlab.core.UnexpectedResponseException: Request url='http://localhost/api/v4/projects/gitlabform_group_negligee%2Fgitlabform_project_zipfile/protected_branches?name=gitlabform_branch_faster', method=POST, json='{"allowed_to_merge": [{"access_level": 30}, {"user_id": 219}, {"user_id": 220}], "allowed_to_push": [{"user_id": 218}, {"access_level": 0}, {"user_id": 220}]}' failed - expected code(s) [200, 201], got code 422 & body: '{"message":["Merge access levels user is not a member of the project","Push access levels user is not a member of the project"]}'

Is it just an isolated local environment issue? Have you run into it before @gdubicki ? If the overall implementation looks good to you, I'll clean things up and add necessary documentations. When you get a chance, please have a look 🙏 .

@amimas amimas temporarily deployed to Integrate Pull Request July 20, 2023 02:40 — with GitHub Actions Inactive
@amimas amimas temporarily deployed to Integrate Pull Request July 20, 2023 15:35 — with GitHub Actions Inactive
@amimas amimas temporarily deployed to Integrate Pull Request July 20, 2023 16:34 — with GitHub Actions Inactive
@amimas amimas marked this pull request as ready for review July 20, 2023 16:35
@amimas amimas temporarily deployed to Integrate Pull Request July 20, 2023 16:54 — with GitHub Actions Inactive
@gdubicki
Copy link
Member

gdubicki commented Jul 20, 2023

There is still one issues remaining. I'm able to get the test pass intermittently. Most of the time it fails with this error Create access levels user is not a member of the project:

I just realized that I run into similar issue locally when I run the tests/acceptance/premium/test_branches.py tests.

gitlabform.gitlab.core.UnexpectedResponseException: Request url='http://localhost/api/v4/projects/gitlabform_group_negligee%2Fgitlabform_project_zipfile/protected_branches?name=gitlabform_branch_faster', method=POST, json='{"allowed_to_merge": [{"access_level": 30}, {"user_id": 219}, {"user_id": 220}], "allowed_to_push": [{"user_id": 218}, {"access_level": 0}, {"user_id": 220}]}' failed - expected code(s) [200, 201], got code 422 & body: '{"message":["Merge access levels user is not a member of the project","Push access levels user is not a member of the project"]}'

Is it just an isolated local environment issue? Have you run into it before @gdubicki ?

Yes, it's some flaky test, don't worry about it.

If the overall implementation looks good to you, I'll clean things up and add necessary documentations. When you get a chance, please have a look 🙏 .

The PR looks great - the docs, the tests and the implementation! I particularly like how clear and easy to understand the tests are. Great job, thank you for your contribution!

For GitLab Premium or Ultimate, tag protection can be done by specifying
individual user or groups. This change adds support for that configuration
by adding 'allowed_to_create' key that takes array of users, groups, and/or
specific role.
@amimas amimas temporarily deployed to Integrate Pull Request July 21, 2023 00:17 — with GitHub Actions Inactive
@amimas amimas temporarily deployed to Integrate Pull Request July 21, 2023 00:27 — with GitHub Actions Inactive
@amimas
Copy link
Collaborator Author

amimas commented Jul 21, 2023

Thanks for the review @gdubicki 🙏

@amimas amimas merged commit c1034b0 into gitlabform:main Jul 21, 2023
23 checks passed
@amimas amimas deleted the issue-505 branch July 21, 2023 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add allowed_to_create support for tag protection
3 participants