Closing per discussion in #592798 (comment 3172099552), will start a new MR for updated approach

What does this MR do and why?

MR adds a new concern that has a web_url and web_path methods. This provides a standardized way for different parts of the application to generate web URLs and paths. Instead of each model calling the URL builder directly, they can now use simple web_url and web_path methods that work consistently across the codebase.

This is an initial iteration of this approach - the new concern is added to the Board and Commit models, and the code is updated to use the new web_url method instead of calling the URL builder directly. There are other outstanding models in lib/gitlab/url_builder.rb - to reduce MR size, they will be iterated in batches.

References

Related to #592798 and effort of https://gitlab.com/gitlab-com/gl-infra/software-delivery/operate/team-tasks/-/work_items/32+

How to set up and validate locally

1. In GDK rails console confirm that URLs are still built correctly

[1] pry(main)> project = Project.first
=> #<Project id:1 toolbox/gitlab-smoke-tests>>
[2] pry(main)> commit = project.repository.commit('HEAD')
=> #<Commit id:79bb5a51635e3f22faf5035e58443a9f59a4ce73 toolbox/gitlab-smoke-tests@79bb5a51635e3f22faf5035e58443a9f59a4ce73>
[3] pry(main)> puts commit.web_url
http://127.0.0.1:3000/toolbox/gitlab-smoke-tests/-/commit/79bb5a51635e3f22faf5035e58443a9f59a4ce73
=> nil
[4] pry(main)> puts commit.web_path
/toolbox/gitlab-smoke-tests/-/commit/79bb5a51635e3f22faf5035e58443a9f59a4ce73
=> nil

1. Visit http://localhost:3000/-/graphql-explorer, validate query against a public project

query {
  project(fullPath: "gitlab-org/gitlab-test") {
    boards(first: 1) {
      nodes {
        id
        name
        webPath
        webUrl
      }
    }
  }
}

{
  "data": {
    "project": {
      "boards": {
        "nodes": [
          {
            "id": "gid://gitlab/Board/1",
            "name": "Development",
            "webPath": "/gitlab-org/gitlab-test/-/boards/1",
            "webUrl": "http://127.0.0.1:3000/gitlab-org/gitlab-test/-/boards/1"
          }
        ]
      }
    }
  },
  "correlationId": "01KKY38EXGCADRSSS094X4B3EB"
}

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Thanks for the clarification on this and reasoning behind this approach 👍 I updated the issue description with this implementation

LGTM, thanks 👍

What does this MR do?

MR extends the AWS Terraform module to support custom security groups on all VM node types, not just gitlab-rails and monitor.

Changes:

• Adds custom_security_group_ids to apply additional Security Groups to all nodes, and *_custom_security_group_ids per-component equivalents for fine-grained control, following the existing custom_tags pattern
• Deprecates gitlab_rails_security_group_ids and monitor_security_group_ids in favour of the new *_custom_security_group_ids naming; existing values are still honoured unless the new variable is set
• Documents the new variables in the "Custom Security Groups" section of environment_advanced_network.md

Related issues

Closes #1223

Author's checklist

When ready for review, the Author applies the workflowready for review label and mention @gitlab-org/software-delivery/get-maintainers:

• Merge request:
  • Corresponding Issue raised and reviewed by the GET maintainers team.
  • Merge Request Title and Description are up-to-date, accurate, and descriptive
  • MR targeting the appropriate branch
  • MR has a green pipeline
  • MR has no new security alerts in the widget from the Secret Detection and IaC Scan (SAST) jobs.
• Code:
  • Check the area changed works as expected across all expected permutations.
  • Check that the changes work across upgrades.
  • Documentation created/updated in the same MR if applicable

Aaah, sorry for the miss. It was weird that only doc change was made 🤔 gitlab-org/frontend/renovate-gitlab-bot!1441 (merged) explains this!

👋 @peterhegman could you please review above and share your thoughts on above failure with Presenter? I am assuming that I miss something here, so appreciate an insight 👀

As I'm onboarding to this area, I relied on DAP and local GDK validation for the changes. So far created draft Draft: Initial standartization of web_url and w... (!227681 - closed) with initial changes, however I believe there's a blocker that affects current proposal implementation.

As part of the above MR and failing jobs there - https://gitlab.com/gitlab-org/gitlab/-/jobs/13529741666 and https://gitlab.com/gitlab-org/gitlab/-/jobs/13529999704. With further Duo help analysis it looks like the challenge is following:

GitLab has three parallel URL approaches in different levels:

1. Presenter layer - Presenter::Base defines web_url and web_path (delegates to Gitlab::UrlBuilder)
2. Model layer - Some models define their own web_url methods such as Organization (direct call to Gitlab::UrlBuilder too)
3. Direct UrlBuilder usage - 200+ locations call Gitlab::UrlBuilder.build() directly (Serializers, Helpers, Views, Models, so on) - example snippet usage or custom commit_url usage

Approach of concern addition to a model, like Commit, as done in !227681 (diffs) results in clash:

• CommitPresenter inherits from Presenter::Delegated (uses SimpleDelegator)
• Presenter::Base already defines web_url and web_path
• Now both the model and presenter have these methods
• The delegator validator flags this as an accidental override https://gitlab.com/gitlab-org/gitlab/-/jobs/13529741666

If models are SSOT for web ul/path and presenters should delegate to models - then to remove above failure would this require refactoring web_url/web_path from Presenter::Base? Then it will affect everything that inherits from it, massive refactor that I'm not seeing how can be broken down so far.