Skip to content

🐛 Fix wrong go-style UUID regexp - made portal links fail #6268

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

Merged

Conversation

mrnicegyu11
Copy link
Member

@mrnicegyu11 mrnicegyu11 commented Aug 29, 2024

What do these changes do?

Portal links opening failed in master.
The reason is that the Traefik-Rule for sending /study/UUID to the webserver would not trigger, as the old-style go-regexp doesnt work anymore in traefik-v3. I have changed the old UUID-catching Regexps to the style of the already updated, already functional one that catches modern (dv2) dynamic services.

I will forcemerge this after some consideration.

Related issue/s

How to test

Dev-ops checklist

@mrnicegyu11 mrnicegyu11 added bug buggy, it does not work as expected a:webserver issue related to the webserver service labels Aug 29, 2024
@mrnicegyu11 mrnicegyu11 added this to the Eisbock milestone Aug 29, 2024
@mrnicegyu11 mrnicegyu11 requested a review from SCA-ZMT August 29, 2024 11:51
@mrnicegyu11 mrnicegyu11 self-assigned this Aug 29, 2024
Copy link

Please retry analysis of this Pull-Request directly on SonarCloud

Copy link

codecov bot commented Aug 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.1%. Comparing base (cafbf96) to head (4a0de8b).
Report is 480 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6268      +/-   ##
=========================================
+ Coverage    84.5%   88.1%    +3.5%     
=========================================
  Files          10    1471    +1461     
  Lines         214   61011   +60797     
  Branches       25    2063    +2038     
=========================================
+ Hits          181   53783   +53602     
- Misses         23    6915    +6892     
- Partials       10     313     +303     
Flag Coverage Δ
integrationtests 64.8% <ø> (?)
unittests 86.1% <ø> (+1.5%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1421 files with indirect coverage changes

@mrnicegyu11 mrnicegyu11 merged commit 7e10cd6 into ITISFoundation:master Aug 29, 2024
41 checks passed
@@ -527,7 +527,7 @@ services:
- traefik.http.routers.${SWARM_STACK_NAME}_legacy_services_catchall.service=${SWARM_STACK_NAME}_legacy_services_catchall
- traefik.http.routers.${SWARM_STACK_NAME}_legacy_services_catchall.priority=1
- traefik.http.routers.${SWARM_STACK_NAME}_legacy_services_catchall.entrypoints=http
- traefik.http.routers.${SWARM_STACK_NAME}_legacy_services_catchall.rule=(Path(`/x/{node_uuid:\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b}`) || PathPrefix(`/x/{node_uuid:\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b}/`))
- traefik.http.routers.${SWARM_STACK_NAME}_legacy_services_catchall.rule=PathRegexp(`\/x\/(?i)[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}`)
Copy link
Member

@pcrespov pcrespov Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrnicegyu11 I propose to create a test in tests/environment-setup/test_traefik_setup.py that checks the regex against some known entrypoints

This is an idea of it ...

import pytest
import re
import yaml

# Function to load the docker-compose file and extract the regex
def extract_regex_from_docker_compose(file_path):
    with open(file_path, 'r') as file:
        compose_data = yaml.safe_load(file)
        labels = compose_data['services']['traefik']['labels']
        for label in labels:
            if 'traefik.http.routers' in label and 'PathRegexp' in label:
                # Extract the regex from the label
                regex = re.search(r'PathRegexp\(`(.*)`\)', label).group(1)
                return regex
    return None

# Test cases with parametrized paths
@pytest.mark.parametrize("path,expected", [
    ("/x/12345678-1234-1234-1234-123456789abc", True),  # Valid UUID
    ("/x/abcdef12-3456-7890-abcd-ef1234567890", True),  # Valid UUID
    ("/x/invalid-uuid-1234-1234-1234-123456789abc", False),  # Invalid UUID
    ("/y/12345678-1234-1234-1234-123456789abc", False),  # Valid UUID but wrong path
    ("/x/1234-5678-1234-1234-123456789abc", False),  # Invalid UUID format
])
def test_traefik_path_regex_matching(path: str, expected: bool):
    # Load the regex from the docker-compose file
    regex = extract_regex_from_docker_compose('docker-compose.yml')
    
    # Compile the regex with the appropriate flags (case insensitive)
    compiled_regex = re.compile(regex, re.IGNORECASE)
    
    # Test if the path matches the regex
    match = compiled_regex.match(path)
    assert (match is not None) == expected

@@ -748,7 +748,7 @@ services:
- traefik.http.services.${SWARM_STACK_NAME}_webserver.loadbalancer.sticky.cookie.secure=true
- traefik.http.middlewares.${SWARM_STACK_NAME}_webserver_retry.retry.attempts=2
- traefik.http.routers.${SWARM_STACK_NAME}_webserver.service=${SWARM_STACK_NAME}_webserver
- traefik.http.routers.${SWARM_STACK_NAME}_webserver.rule=(Path(`/`) || Path(`/v0`) || Path(`/socket.io/`) || Path(`/static-frontend-data.json`) || Path(`/study/{study_uuid:\b[0-9a-f]{8}\b-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-\b[0-9a-f]{12}\b}`) || Path(`/view`) || Path(`/#/view`) || Path(`/#/error`) || PathPrefix(`/v0/`))
- traefik.http.routers.${SWARM_STACK_NAME}_webserver.rule=(Path(`/`) || Path(`/v0`) || Path(`/socket.io/`) || Path(`/static-frontend-data.json`) || PathRegexp(`\/study\/(?i)[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}`) || Path(`/view`) || Path(`/#/view`) || Path(`/#/error`) || PathPrefix(`/v0/`))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same story here. We need a way to reliably enforce certain conditions and avoid errors

mrnicegyu11 added a commit to mrnicegyu11/osparc-simcore that referenced this pull request Aug 29, 2024
sanderegg pushed a commit that referenced this pull request Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service bug buggy, it does not work as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants