Skip to content

Conversation

@anon-pradip
Copy link
Contributor

@anon-pradip anon-pradip commented Dec 15, 2025

Description

Key Changes

1. CI Pipeline Configuration (.drone.star)

  • Enabled k8s for wopi tests: Set "k8s": True and "skip": False for the wopi test configuration targeting the apiCollaboration suite
  • Set "collaborationServiceNeeded": True to ensure WOPI services are deployed
  • testOcis function: Added appregistry readiness checks and config file validation to prevent race conditions:
    "kubectl -n ocis wait --for=condition=ready pod -l app=appregistry --timeout=60s",
    "kubectl -n ocis exec $(kubectl -n ocis get pod -l app=appregistry -o jsonpath='{.items[0].metadata.name}') -- test -f /etc/ocis/app-registry.yaml && echo 'Config file exists' || (echo 'Config file missing'; exit 1)",

2. WOPI Configuration and Setup

  • injectWopiConfig(): Injects WOPI configuration into deployment-values.yaml including:
    • Office suites: FakeOffice, Collabora, OnlyOffice
    • MIME type mappings with default app assignments (e.g., .odt → FakeOffice)
    • WOPI source URLs and postMessage origins
  • patchCollaborationWopiSrc(): Updates Helm templates to use dynamic wopiSrc values from configuration
  • Port forwarding setup: Added getWopiPortForwardCommands() for exposing collaboration services on localhost ports (9300, 9302, 9304, 9305)

3. Feature File Corrections

  • wopi.feature: Updated WOPI URL regex patterns to accept URLs ending with either $ or & for better compatibility
  • checkFileInfo.feature: Changed PostMessageOrigin from hardcoded localhost to dynamic %base_url% pattern

4. Kubernetes Infrastructure

  • FakeOffice service manifests: Added deployment and service YAML files for the FakeOffice WOPI server in k8s
  • Hosting discovery configuration: Enhanced hosting-discovery.xml with comprehensive file extension support and template conversion actions

5. Test Environment Setup

  • Deployment preparation: Modified prepareOcisDeployment() to handle WOPI-specific configurations
  • Service exposure: Added functions to expose collaboration services via NodePort for external access

Technical Details

  • Race condition mitigation: Added explicit waits for appregistry pod readiness and config file presence
  • Dynamic configuration: WOPI settings are injected at deployment time rather than hardcoded
  • Port management: Proper port forwarding for multiple WOPI services (FakeOffice, Collabora, OnlyOffice)
  • MIME type handling: Comprehensive mapping of office document types to appropriate WOPI applications

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@anon-pradip anon-pradip self-assigned this Dec 15, 2025
@anon-pradip anon-pradip force-pushed the test/include-wopi-test-suite-in-k8s branch 28 times, most recently from 9f0c580 to 7624c85 Compare December 18, 2025 06:50
@anon-pradip anon-pradip force-pushed the test/include-wopi-test-suite-in-k8s branch 4 times, most recently from 6a83848 to 2140441 Compare January 6, 2026 04:07
@anon-pradip anon-pradip force-pushed the test/include-wopi-test-suite-in-k8s branch 3 times, most recently from 4358f96 to d8b142d Compare January 13, 2026 08:34
@anon-pradip anon-pradip marked this pull request as ready for review January 13, 2026 08:37
@anon-pradip anon-pradip requested review from Ashim-Stha and removed request for individual-it, kobergj and phil-davis January 13, 2026 08:38
@anon-pradip anon-pradip force-pushed the test/include-wopi-test-suite-in-k8s branch from d8b142d to 8a68e18 Compare January 13, 2026 11:35
.drone.star Outdated
Comment on lines 744 to 749
"export KUBECONFIG=%s/kubeconfig-$${DRONE_BUILD_NUMBER}.yaml" % dirs["base"],
"until test -f $${KUBECONFIG}; do sleep 1; done",
] + getWopiPortForwardCommands() + [
"sleep 5",
"kubectl -n ocis wait --for=condition=ready pod -l app=appregistry --timeout=60s",
"kubectl -n ocis exec $(kubectl -n ocis get pod -l app=appregistry -o jsonpath='{.items[0].metadata.name}') -- test -f /etc/ocis/app-registry.yaml && echo 'Config file exists' || (echo 'Config file missing'; exit 1)",
Copy link
Member

Choose a reason for hiding this comment

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

we do not need these commands here. this is a wrong place. 😅

Copy link
Member

Choose a reason for hiding this comment

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

This is causing the unit test pipeline to run infinitely

def getWopiPortForwardCommands():
"""Get port forwarding commands for WOPI collaboration services"""
return [
"bash -lc 'command -v kubectl >/dev/null || (curl -fsSL -o /usr/local/bin/kubectl https://dl.k8s.io/release/v1.27.3/bin/linux/amd64/kubectl && chmod +x /usr/local/bin/kubectl) || true'",
Copy link
Member

Choose a reason for hiding this comment

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

you do that in two places, and the version is hard-coded in two places. If one day someone wants to change the version number, chances are big that only one will be changed and that will lead to a lot of debugging time wasted

"sed -i 's|name: sharing-banned-passwords-{{ .appName }}|name: sharing-banned-passwords|' ./charts/ocis/templates/frontend/deployment.yaml",
# Add OCIS_CONFIG_DIR env var to appregistry deployment
"grep -q 'name: OCIS_CONFIG_DIR' ./charts/ocis/templates/appregistry/deployment.yaml || " +
"cat > /tmp/patch_appregistry.py << 'EOF'" + "\n" +
Copy link
Member

Choose a reason for hiding this comment

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

I find it super confusing if the file is created here and then executed.
I think usually we save these kind of tools in a separate files and then execute them.
Or what about even not patching the deployment files, but just having a separate deployment.yaml that is already pre-patched?

json.dump(data, f, indent=2)
PYEOF"""

def injectWopiConfig():
Copy link
Member

Choose a reason for hiding this comment

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

we need a different solution here. Finding or changing something in that endless awk line is 🤯

@anon-pradip anon-pradip force-pushed the test/include-wopi-test-suite-in-k8s branch from 8a68e18 to 561a029 Compare January 26, 2026 12:15
@anon-pradip anon-pradip force-pushed the test/include-wopi-test-suite-in-k8s branch from 561a029 to fb8c653 Compare January 26, 2026 12:23
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants