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

Resource Groups - don't fail if key not exist #649

Closed
philippe-granet opened this issue Dec 2, 2023 · 10 comments · Fixed by #650 or #756
Closed

Resource Groups - don't fail if key not exist #649

philippe-granet opened this issue Dec 2, 2023 · 10 comments · Fixed by #650 or #756
Labels
✨feature request gitlab-free This feature would support GitLab Free and above tiers good first issue Good issue for first-time contributors help wanted

Comments

@philippe-granet
Copy link
Contributor

I have hundred of Gitab projects, some of them have a ressource group name 'bot-pipeline', but lots of them don't.
And declare in config.yml all projects that have this group name is cumbersome.

Is it possible to add an option on Resource groups to skip Gitlab projects for which a ressource group key defined in config.yml doesn't exist (ignore 404 error returned by Gitlab API)?

@amimas
Copy link
Collaborator

amimas commented Dec 2, 2023

Yes that can be useful. Do you have any suggestion for the config syntax? Will you be able to help contribute this feature?

@amimas amimas added ✨feature request gitlab-free This feature would support GitLab Free and above tiers good first issue Good issue for first-time contributors help wanted labels Dec 2, 2023
@philippe-granet
Copy link
Contributor Author

philippe-granet commented Dec 5, 2023

@amimas Something like an optional attribute fail_if_not_exist, default to true ?

projects_and_groups:
  group_1/project_1:
    resource_groups:
      staging:
        process_mode: oldest_first
      production: 
        process_mode: newest_first
      resource_group_that_dont_exist:
        process_mode: newest_first
        fail_if_not_exist: false

@amimas
Copy link
Collaborator

amimas commented Dec 7, 2023

Hi @philippe-granet - thanks for the suggestion.

Does the optional attribute need to be for each resource group? What if it's a direct child key of resource_groups?

One of the design patterns gitlabform follows is raw parameter passing. So if gitlab adds/updates any attributes to the api for managing individual resource group, you can simply add that in the config without waiting for gitlabform to add support for it. See this doc:

https://github.com/gitlabform/gitlabform/blob/main/docs/reference/index.md#raw-parameters-passing

That's why I think it's best to not add the optional attribute under specific resource group and I don't see any significant benefit that way either.

What do you think? If you agree, the revised config proposal could be like this:

projects_and_groups:
  group_1/project_1:
    resource_groups:
      fail_if_not_exist: false
      staging:
        process_mode: oldest_first
      production: 
        process_mode: newest_first
      resource_group_that_dont_exist:
        process_mode: newest_first

Also, a minor feedback: I wonder if the optional attribute can have a different name. It feels too verbose or unclear why it's needed unless the author is famiar with Gitlab's behaviour. I can't think of a suggestion either.

@philippe-granet
Copy link
Contributor Author

philippe-granet commented Dec 7, 2023

@amimas ok for direct child key of resource_groups
But I also see that you use a CLI parameter --strict for https://gitlabform.github.io/gitlabform/reference/files/

only_first_branch - if set to true then only the first branch from the list above that exists will be processed (unless you pass --strict as cli parameter to the app - then it will fail when trying to process a non-existent branch),

Our case is similar, we want to fail (or not) when trying to process a non-existent resource group.
Should it be also a CLI parameter?

For the name, yes I think we could find a better name. Some propositions:

  • strict
  • ensure_exist
  • verify_exist
  • check_exist
  • check_or_fail
  • verify_or_fail
  • ensure_or_fail

@amimas
Copy link
Collaborator

amimas commented Dec 9, 2023

Good call about the --strict CLI parameter. I haven't used that yet and I'm kinda on the fence. Maybe that's because not all api endpoints or gitlab feature behave the same way. For example, it's possible to configure protection for non-existent branch.

I think I like it as a config key instead of CLI parameter. That way source of truth is in the config file as opposed to how it was executed. But that may not help everyone. I like how renovate manages all their options. Every configuration is available as CLI parameter, or config key, or environment variable.

Also I think it can be expected that you may want one config to be strict while other one is not. So, CLI probably isn't the best. What do you think?

Gitlabform doesn't do this yet, but maybe here strict can be used as both CLI parameter and/or config key for resource_group. That way it's more flexible for the community.

If we don't go with strict, maybe ensure_exists also make sense or is intuitive.

It'd be goood to get some feedback from @gdubicki as he created the tool and may have more insights.

@amimas
Copy link
Collaborator

amimas commented Jan 4, 2024

@philippe-granet - thoughts on the above ☝️ ?

@jimisola - what do you think?

@jimisola
Copy link
Collaborator

jimisola commented Jan 4, 2024

I ended up in the deep end directly here. FYI, I'm not really familiar with the code base of gitlabform. @gdubicki was "kind" and trusted me after I spent quite some time on the structure of our next version on the yaml configuration.

I think the solution that you two have come up with is good enough (direct child key of resource_groups).
I prefer ensure_exists. Do we have any similar cases? Good if we could use the same naming standard.

That said, I never used resource groups and I'm trying to understand the actual problem. Why is an 404 error returned by GitLab API when the resource group does not exist?

I thought that the resource groups are created for project_1 using:

projects_and_groups:
  group_1/project_1:
    resource_groups:
      fail_if_not_exist: false
      staging:
        process_mode: oldest_first
      production: 
        process_mode: newest_first
      resource_group_that_dont_exist:
        process_mode: newest_first

But, as I understand it - if resource_group_that_dont_exist does not exist for project_1 then an 404 error is returned. So, my question is then: where are the resource groups defined? Does the resource group have to match the environment in jobs for project_1?

@amimas
Copy link
Collaborator

amimas commented Jan 4, 2024

@jimisola - The resource group is only created when it's used in a project's .gitlab-ci.yml file using resource_group key in a job. And you can't configure processing mode there unfortunately.

Right now processing mode can only be set via API. That's why when trying to configure/set a resource group's "processing mode" it has to first exist; otherwise you get 404 not found error.

@amimas
Copy link
Collaborator

amimas commented Jan 9, 2024

@philippe-granet - are you able to continue with this based on above discussion?

philippe-granet added a commit to philippe-granet/gitlabform that referenced this issue Jan 13, 2024
@philippe-granet
Copy link
Contributor Author

@amimas I have updated the PR #650 based on above discussion

philippe-granet added a commit to philippe-granet/gitlabform that referenced this issue Jan 18, 2024
amimas added a commit to philippe-granet/gitlabform that referenced this issue May 11, 2024
amimas added a commit to philippe-granet/gitlabform that referenced this issue May 15, 2024
TimKnight-DWP pushed a commit to philippe-granet/gitlabform that referenced this issue May 15, 2024
gdubicki pushed a commit that referenced this issue May 15, 2024
…ist (#650)

Fix #649

Co-authored-by: amimas <aver.mimas@gmail.com>
mahadevan-karthi-dwp added a commit to mahadevan-karthi-dwp/gitlabform that referenced this issue May 15, 2024
feat: fix acceptance test

feat: black format

feat: delete test file

feat: delete test file

fix: add config and modify docker shellscript

fix: add saml config

chore: remove mypy pre-commit

- in pre-commit it is run in isolated environment
- often has issues with python-gitlab types
- wants to scan everything not just changed files
- quickly becoming a hinderance not a help

Signed-off-by: Tim Knight <tim.knight1@engineering.digital.dwp.gov.uk>

feat: add option to not fail if configured resource group does not exist (gitlabform#650)

Fix gitlabform#649

Co-authored-by: amimas <aver.mimas@gmail.com>

fix: review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨feature request gitlab-free This feature would support GitLab Free and above tiers good first issue Good issue for first-time contributors help wanted
Projects
None yet
3 participants