[go: up one dir, main page]
Skip to content

[Spike] Pages Validation: Review Apps for GitLab Pages

This is a spike to enable the completion of Pages Validation: Review Apps for GitLab Pages (#16907 - closed) in a future milestone.

Details

These questions an proposed solutions based on my work on the PoC: Draft: [PoC] Pages merge request preview (!122498 - closed).

Screen_Recording_2023-06-15_at_09.16.00

What unknowns or questions need to be answered to deliver Review Apps for GitLab Pages?
What could be the name of this feature?

This is an important question, and it's related to another question that I heard:

Some projects already have Review Apps which gives access to their pages, what's the difference here?

In my opinion, to avoid confusion with the review app feature, and to make the name of the feature reflect more explicitly its intent, I'd name it something like Pages merge request preview. The main difference between a review app and a pages merge request preview is that the latter doesn't require a dynamic environment, instead the it uses the same GitLab Pages infrastructure to serve the a GitLab Pages site built in the pages:preview ci job on a Merge Request pipeline.

Will this be a premium feature?

I think this has the potential to be a premium feature.

Overview: How would this feature work?

Projects would have a pages:preview merge request job in their merge request pipelines, which would create a PagesDeployment that would be accessible through a temporary random-ish URL with the intent of reviewing changes proposed on GitLab Pages sites. The temporary random-ish URL would be exposed during the build through a new predefined environment variable $CI_PAGES_MERGE_REQUEST_PREVIEW_URL. Example:

pages:preview:
  stage: deploy
  script:
  - 'echo "Preview url: "$CI_PAGES_MERGE_REQUEST_PREVIEW_URL"'
  artifacts:
    paths:
    - public
  only:
  - merge_requests
What format the preview URL should have?

Based on the discussions on the original issue, I suggest creating slugs of 63 chars long, which is the limit for a subdomain label (https://www.freesoft.org/CIE/RFC/1035/9.htm), in the following format: preview*<PROJECT PATH (up to 23 chars)>*<RANDOM STRING>. This way the final url might look like: preview*gitlab*c589da2bd33208d7457bdf650976501b232cc0734dd620a5.gitlab.io

  • Why use *?

Short answer: to prevent preview URL hijack.

If we use - instead of *, like: preview-<PROJECT PATH (up to 23 chars)>-<RANDOM STRING>, a user could hijack the preview traffic by creating a project with that name. For example if a MR generates the following preview URL: preview-gitlab-c589da2bd33208d7457bdf650976501b232cc0734dd620a5.gitlab.io, a user could use that same value as the project name, and that would have precedence on the pages lookup, hijacking that traffic.

Therefore, we can use a separator that's a valid URL char, but it's not valid in a project path/name.

What could invalidate the preview?

Initially I though that we could invalidate/delete the previews when the Merge Request were merged or closed. But, after I checked the oldest opened MR in gitlab-org/gitlab is from 2017, I now also think that we could have a time-based invalidation to avoid keeping old objects in the object storage, and also reduce possible random URLs, for old builds. With that in mind, I'd propose to keep the merge request pages preview up to 30 days after it's built.

What architectural or other decisions need to be made?
Should we create a new database table to handle the preview deployments and slug or should we use any of the existing pages tables?
  • Can we use the pipeline artifact directly without a new PagesDeployment object?
  • Can we add the temporary slug/url for the pages preview in the PagesDeployment table?

Given the volume of MRs some projects have, I think that having a database table to handle the association between Pages deployments and the merge requests would be a good practice. This will permit us to have a small model where we can keep the logic to create the temporary slug and some validations. I'd keep using the PagesDeployment to keep track of the deployment itself. That already have all the validations required regarding how to build and keep the deployment and it's used to build the internal API (api/internal/pages) response. In the PoC, I just used it as it's, but if we keep this strategy we might want to add a preview? field there to make it easier to differentiate preview deploys from production deploys.

I tried to use some of the existing Pages deployment logic during the PoC. But, should I? Or should this feature use its own architecture?

During the PoC re-using some/most of the existing Pages logic was very helpful and make the overall change small (given that we're introducing a new feature). We might want to slightly refactor parts of the existing code to improve the extensibility, but I don't think there's much to change to adapt it to the new feature.

When invalidating/deleting the preview, should we use the Gitlab EventStore?

Gitlab EventStore is the base of our new-ish event oriented architecture. It's already being used on some new MergeRequest related features ( !92520 (merged)), so I'd use it to handle the merge request merged/closed events.

The pages preview requires two main record informations, What should be the lifetime for each of this?

  1. the pages deployment: the site itself

    Could be invalidated with any new Merge Request CI build.

  2. the preview temporary URL/slug

    Could be invalidated only when the MR is merged or closed. This way the same URL/slug could be used between different MR builds, which IMO might make the feature usage easier and contributors could share these URLs while the MR is open.

But, as mentioned above, I think both should have an specific temporal based lifetime as well to avoid overusing the object storage and keeping possible random slugs.

How should the temporary URL be exposed to users?

  • During the build

    In the build we could expose the URL using a predefined variable like $CI_PAGES_MERGE_REQUEST_PREVIEW_URL

  • In the UI

    I propose to add a new row in the pipeline widget on the MR description tab to expose the preview URL to the users. Something like: \

ui

Which specializations are required to complete the work? (e.g. Backend, Frontend, UX, Docs)

This will be mostly a backend change, but we'll also need a frontend to expose the preview URL in the MR UI. Technical Writing will also be required as the new feature will require some customer facing documentation.

Acceptance Criteria

The outcome of this spike is one or more issues, with estimated weight, that can be scheduled in future milestones and result it in the delivery of Review Apps for GitLab Pages. The scope should be agreed between Engineering and Product.

Initial proposed list of issues canceled due to new information

  1. Pages merge request preview: backend base ( backend | weight: 3~5)

    This would be the database migrations, feature flag, if the feature is only premium add the availability check and create the temporary URL/slug and association with the PagesDeployment. This would probably be more than 1 merge request, but I think it might be scoped in one issue.

  2. Pages merge request preview: invalidation of previews ( backend | weight: 3)

  3. Pages merge request preview: expose the temporary URL in the CI build ( backend | weight: 2)

  4. Pages merge request preview: expose the temporary URL in the UI ( backend | weight: 3)

  5. Pages merge request preview: expose the temporary URL in the UI ( frontend | weight: ?)

  6. Pages merge request preview: customer documentation ( Technical Writing | weight: ?)

Edited by Kassio Borges