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.

Sign up for GitHub

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

Fix malformed caddyfile #260

Merged
merged 2 commits into from
Feb 7, 2025
Merged

Fix malformed caddyfile #260

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4794242
Fix malformed caddyfile
florkbr Feb 7, 2025
046f09e
Code review comments to place caddyfile overwrites behind FE env bool
florkbr Feb 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions controllers/reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,26 @@ func (r *FrontendReconciliation) populateEnvVars(d *apps.Deployment, frontendEnv
})
}

envVars = append(envVars, v1.EnvVar{
Name: "APP_NAME",
Value: r.Frontend.Name,
})

envVars = append(envVars, v1.EnvVar{
Name: "ROUTE_PATH",
Value: "/apps/$(APP_NAME)",
})

envVars = append(envVars, v1.EnvVar{
Name: "BETA_ROUTE_PATH",
Value: "/beta$(ROUTE_PATH)",
})

envVars = append(envVars, v1.EnvVar{
Name: "PREVIEW_ROUTE_PATH",
Value: "/preview$(ROUTE_PATH)",
})

d.Spec.Template.Spec.Containers[0].Env = envVars
}

Expand Down
40 changes: 36 additions & 4 deletions controllers/templates/Caddyfile
View file
Open in desktop
Copy link
Contributor Author

@florkbr florkbr Feb 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I did a local diff of the Caddyfile in this PR versus what's in frontend-build-container:3bacc0b:

➜  frontend-operator git:(fix-malformed-caddyfile) ✗ diff controllers/templates/Caddyfile_from_3bacc0b controllers/templates/Caddyfile
2c2
<     {\$CADDY_TLS_MODE}
---
>     {$CADDY_TLS_MODE}
12c12
<   
---
> 
14c14,15
<     {\$CADDY_TLS_CERT}
---
>     {$CADDY_TLS_CERT}
>     header -Vary
19c20
<         path ${ROUTE_PATH}*
---
>         path {$ROUTE_PATH}*
22c23
<         uri strip_prefix ${ROUTE_PATH}
---
>         uri strip_prefix {$ROUTE_PATH}
24c25
<             root /opt/app-root/src/${DIST_FOLDER}$([ "$IS_PR" == true ] && echo "" || echo "/stable")
---
>             root /opt/app-root/src/dist/stable
31c32
<         path ${BETA_ROUTE_PATH}*
---
>         path {$BETA_ROUTE_PATH}*
34c35
<         uri strip_prefix ${BETA_ROUTE_PATH}
---
>         uri strip_prefix {$BETA_ROUTE_PATH}
36c37
<             root /opt/app-root/src/${DIST_FOLDER}$([ "$IS_PR" == true ] && echo "" || echo "/preview")
---
>             root /opt/app-root/src/dist/preview
43c44
<         path ${PREVIEW_ROUTE_PATH}*
---
>         path {$PREVIEW_ROUTE_PATH}*
46c47
<         uri strip_prefix ${PREVIEW_ROUTE_PATH}
---
>         uri strip_prefix {$PREVIEW_ROUTE_PATH}
48c49
<             root /opt/app-root/src/${DIST_FOLDER}$([ "$IS_PR" == true ] && echo "" || echo "/preview")
---
>             root /opt/app-root/src/dist/preview

As far as I can tell - only chrome specifies anything other than dist for the DIST_FOLDER (and we currently have an exception for chrome in the reconciler). See https://github.com/search?q=org%3ARedhatInsights+path%3A**%2Fbuild_deploy.sh+DIST_FOLDER&type=code&ref=advsearch

I was unclear on how to map $IS_PR to the FEO environment.

There does appear to be a few frontends that specify a custom Caddyfile. See https://github.com/search?q=org%3ARedhatInsights+path%3A**%2FCaddyfile&type=code&ref=advsearch

I can remove the beta/preview paths - but figured it was best to stay consistent with what we have today and remove later.

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{$CADDY_TLS_MODE}
auto_https disable_redirects
servers {
metrics
metrics
}
}

Expand All @@ -17,16 +17,48 @@

# Handle main app route
@app_match {
path ${ROUTE_PATH}*
path {$ROUTE_PATH}*
}
handle @app_match {
uri strip_prefix ${ROUTE_PATH}
uri strip_prefix {$ROUTE_PATH}
file_server * {
root /srv/${OUTPUT_DIR}
root /opt/app-root/src/dist/stable
browse
}
}

# Handle beta app route
@beta_match {
path {$BETA_ROUTE_PATH}*
}
handle @beta_match {
uri strip_prefix {$BETA_ROUTE_PATH}
file_server * {
root /opt/app-root/src/dist/preview
browse
}
}

# Handle preview app route
@preview_match {
path {$PREVIEW_ROUTE_PATH}*
}
handle @preview_match {
uri strip_prefix {$PREVIEW_ROUTE_PATH}
file_server * {
root /opt/app-root/src/dist/preview
browse
}
}

handle /beta/ {
redir /beta/apps/chrome/index.html permanent
}

handle /preview/ {
redir /preview/apps/chrome/index.html permanent
}

handle / {
redir /apps/chrome/index.html permanent
}
Expand Down
8 changes: 8 additions & 0 deletions tests/e2e/http_headers/02-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ spec:
X-Frame-Options Set
X-XSS-Protection 1; mode=block;
}
- name: APP_NAME
value: chrome
- name: ROUTE_PATH
value: /apps/$(APP_NAME)
- name: BETA_ROUTE_PATH
value: /beta$(ROUTE_PATH)
- name: PREVIEW_ROUTE_PATH
value: /preview$(ROUTE_PATH)
ports:
- name: web
containerPort: 80
Expand Down
8 changes: 8 additions & 0 deletions tests/e2e/ssl/02-assert.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ spec:
value: https_port 8000
- name: CADDY_TLS_CERT
value: tls /opt/certs/tls.crt /opt/certs/tls.key
- name: APP_NAME
value: chrome
- name: ROUTE_PATH
value: /apps/$(APP_NAME)
- name: BETA_ROUTE_PATH
value: /beta$(ROUTE_PATH)
- name: PREVIEW_ROUTE_PATH
value: /preview$(ROUTE_PATH)
volumeMounts:
- name: config
mountPath: /opt/app-root/src/build/stable/operator-generated
Expand Down
Loading