Conversation
* Initial plan * Add comprehensive Copilot instruction files Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add README and validation script for Copilot instructions Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
…ized auth (#93) * Initial plan * Implement ingress-authz-overlay chart per issue #91 Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Address code review: remove unused helpers, add container security context Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add Let's Encrypt / cert-manager documentation to user guide Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
|
How is letsencrypt-prod-account-key and letsencrypt-staging-account-key set? |
* Initial plan * Add documentation for Let's Encrypt ACME account key secrets Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add cert-manager installation instructions and troubleshooting Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Fix installation order list formatting Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add troubleshooting for Helm ownership conflict with ClusterIssuer Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Improve Helm ownership conflict documentation clarity Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
|
Getting error """ |
…rlay (#96) * Initial plan * Fix cert-manager certificate ownership conflict by using primary route flag Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
|
Examine ingress configuration:
Why are there two running? |
* Initial plan * Configure overlay to use centralized authz-adapter in security namespace Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
|
@copilot """ |
|
https://gh.io/copilot-coding-agent-docs |
…figuration documentation (#98) * Initial plan * Add Ingress and Connectivity troubleshooting section to docs Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add AWS EKS and on-premises ingress configuration docs Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Fix code review issues in ingress docs Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
|
github-repo-registrations-eventsource has no ingress CLASS assigned """ |
… cert-manager issues (#101) * Initial plan * Add troubleshooting docs for external connection issues when internal services work Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add kind cluster and cert-manager troubleshooting to docs Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add DNS-01 challenge docs for No-IP.com and kind clusters Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
|
@copilot |
…, and NodePort external IP configuration to troubleshooting docs (#102) * Initial plan * Add comprehensive DNS-01 challenge debugging guide with propagation error fixes Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add manual certificate installation guide to troubleshooting docs Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
|
""" |
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive ingress overlay system for the Argo Stack, providing unified, path-based routing with centralized authorization for multi-tenant UIs and APIs. The implementation includes extensive documentation, test specifications, and GitHub Copilot instruction files to guide future development.
Key Changes
- Ingress AuthZ Overlay: New Helm overlay chart providing single-host, path-based ingress for all services (
/workflows,/applications,/registrations,/api,/tenants) with centralized auth viaauthz-adapter - Cross-Namespace Routing: Support for routing ingress traffic across namespaces using ExternalName services
- Comprehensive Documentation: Added 1,800+ lines of troubleshooting guide covering ingress, TLS certificates, DNS, and environment-specific configurations (AWS EKS, bare metal)
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
helm/argo-stack/values.yaml |
Added ingressAuthzOverlay configuration block with route definitions for all services |
helm/argo-stack/overlays/ingress-authz-overlay/values.yaml |
Complete overlay values with auth adapter config, TLS settings, and route specifications |
helm/argo-stack/overlays/ingress-authz-overlay/templates/*.yaml |
Helm templates for ingress resources, ExternalName services, and auth adapter deployment |
helm/argo-stack/overlays/ingress-authz-overlay/docs/authz-ingress-user-guide.md |
586-line user guide covering installation, cert-manager setup, and troubleshooting |
docs/troubleshooting.md |
Added 1,700+ lines of ingress troubleshooting content including connection issues, certificate management, and environment-specific configurations |
Makefile |
Updated ports target to deploy ingress overlay and configure NodePort for kind clusters |
kind-config.yaml |
Added port mappings for HTTP (30080→80) and HTTPS (30443→443) |
.github/instructions/*.md |
Added GitHub Copilot instruction files for Python, Helm/Kubernetes, Docker, and Bash |
QUICKSTART.md, CONTRIBUTING.md |
Restructured documentation removing redundant content |
ORPHAN_ANALYSIS.md, IMPLEMENTATION_SUMMARY.md, FINAL_SUMMARY.md, CLEANUP_SUMMARY.md |
Removed old analysis documents (cleanup from previous work) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # --set ingressAuthzOverlay.enabled=true | ||
|
|
||
| ingressAuthzOverlay: | ||
| enabled: true |
There was a problem hiding this comment.
The ingressAuthzOverlay.enabled is set to true by default in the main values.yaml. This could cause issues for users who haven't configured the overlay or don't want to use it. Consider setting this to false by default to make it opt-in, which is safer and follows the principle of least surprise.
| enabled: true | |
| enabled: false |
|
|
||
| ingressAuthzOverlay: | ||
| enabled: true | ||
| host: calypr-demo.ddns.net |
There was a problem hiding this comment.
The host value calypr-demo.ddns.net appears to be an environment-specific value. This should either be left empty with a comment indicating it must be set, or use a placeholder like example.com. Having a specific environment value in the default configuration could lead to confusion or misconfigurations.
| host: calypr-demo.ddns.net | |
| # Set this to the external DNS host for your deployment environment. | |
| host: example.com |
| # {{- if and $config.tls.enabled $route.primary }} | ||
| # # Let's Encrypt / cert-manager integration (only on primary route to avoid ownership conflicts) | ||
| # cert-manager.io/cluster-issuer: {{ $config.tls.clusterIssuer | quote }} | ||
| # {{- end }} |
There was a problem hiding this comment.
There's commented-out code for cert-manager cluster-issuer annotation (lines 44-47). If this functionality is intentionally disabled, consider removing it entirely or documenting why it's commented out. If it's meant to be used, it should be uncommented and properly configured.
| # {{- if and $config.tls.enabled $route.primary }} | |
| # # Let's Encrypt / cert-manager integration (only on primary route to avoid ownership conflicts) | |
| # cert-manager.io/cluster-issuer: {{ $config.tls.clusterIssuer | quote }} | |
| # {{- end }} | |
| {{- if and $config.tls.enabled $route.primary }} | |
| # Let's Encrypt / cert-manager integration (only on primary route to avoid ownership conflicts) | |
| cert-manager.io/cluster-issuer: {{ $config.tls.clusterIssuer | quote }} | |
| {{- end }} |
Makefile
Outdated
| sudo cp /etc/letsencrypt/live/calypr-demo.ddns.net/fullchain.pem /tmp/ | ||
| sudo cp /etc/letsencrypt/live/calypr-demo.ddns.net/privkey.pem /tmp/ | ||
| sudo chmod 644 /tmp/fullchain.pem /tmp/privkey.pem | ||
| kubectl create secret tls calypr-demo-tls -n argo-stack --cert=/tmp/fullchain.pem --key=/tmp/privkey.pem || true | ||
| kubectl create secret tls calypr-demo-tls -n argocd --cert=/tmp/fullchain.pem --key=/tmp/privkey.pem || true | ||
| kubectl create secret tls calypr-demo-tls -n argo-workflows --cert=/tmp/fullchain.pem --key=/tmp/privkey.pem || true | ||
| kubectl create secret tls calypr-demo-tls -n argo-events --cert=/tmp/fullchain.pem --key=/tmp/privkey.pem || true | ||
| kubectl create secret tls calypr-demo-tls -n calypr-api --cert=/tmp/fullchain.pem --key=/tmp/privkey.pem || true | ||
| kubectl create secret tls calypr-demo-tls -n calypr-tenants --cert=/tmp/fullchain.pem --key=/tmp/privkey.pem || true | ||
| kubectl create secret tls calypr-demo-tls -n default --cert=/tmp/fullchain.pem --key=/tmp/privkey.pem || true | ||
| # install ingress | ||
| helm upgrade --install ingress-authz-overlay \ | ||
| helm/argo-stack/overlays/ingress-authz-overlay \ | ||
| --namespace argo-stack \ | ||
| --set ingressAuthzOverlay.host=${ARGO_HOSTNAME} | ||
| # start nginx | ||
| helm repo add ingress-nginx https://kubernetes.github.io/ingress-nginx | ||
| helm repo update | ||
| helm upgrade --install ingress-nginx ingress-nginx/ingress-nginx \ | ||
| -n ingress-nginx --create-namespace \ | ||
| --set controller.service.type=NodePort \ | ||
| --set controller.extraArgs.default-ssl-certificate=default/calypr-demo-tls | ||
| kubectl create secret tls calypr-demo-tls -n ingress-nginx --cert=/tmp/fullchain.pem --key=/tmp/privkey.pem || true | ||
| sudo rm /tmp/fullchain.pem /tmp/privkey.pem | ||
| # Assign external address | ||
| kubectl patch svc ingress-nginx-controller -n ingress-nginx -p '{ "spec": { "type": "NodePort", "externalIPs": ["100.22.124.96"] } }' |
There was a problem hiding this comment.
The Makefile ports target contains hardcoded environment-specific values:
- Domain:
calypr-demo.ddns.net(lines 163-164, 185) - IP address:
100.22.124.96(line 188)
These should be replaced with variables (e.g., ${ARGO_HOSTNAME} for the domain and a configurable ${EXTERNAL_IP} variable for the IP address). The hardcoded values will not work for users deploying to their own environments.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| namespace: metallb-system | ||
| spec: | ||
| addresses: | ||
| - 100.22.124.96-100.22.124.96 # Adjust to your available IP range |
There was a problem hiding this comment.
The IP address 100.22.124.96 is hardcoded and appears to be environment-specific. While there's a comment saying "Adjust to your available IP range", this file is in the repository and will be deployed as-is by users who may not read the comment. Consider:
- Using a more obviously placeholder range like
192.168.1.240-192.168.1.240 - Adding a more prominent warning at the top of the file
- Moving this to an example file rather than a directly deployable overlay
|
Examine ingress all of these routes return a 404:
|
* Initial plan * Fix 404 errors: add backend-protocol support for HTTPS backends and improve route defaults Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add valid values comment for backendProtocol annotation Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * fix ingress --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> Co-authored-by: Brian Walsh <brian@bwalsh.com>
* adds testing values * Add tests and helm chart support for debug mode behavior (#107) * Initial plan * Initial plan Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add tests for get_debugging_vars() and check() with DEBUG_EMAIL/DEBUG_GROUPS env vars Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Update docs/testing.md with authz-adapter testing documentation Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add DEBUG_EMAIL and DEBUG_GROUPS environment variable support to helm chart Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
* adds user guide * work git app (#110) * fix authz image * ensure web apps load from /path * adds roles overlay * refactor roles into main chart * refactor roles into main chart * adjust app name and secret keys * adds admin roles * WIP draft gitapp docs * adds dex config * adds argocd dex.config for authproxy * Add markdown landing page feature for authz-adapter (#114) * Initial plan * Add landing page feature with markdown rendering (#113) Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Address code review: add security hardening for landing page Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Fix path traversal protection with pathlib and simplify file serving Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Refactor: Extract landing page as standalone service Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Revert "Add markdown landing page feature for authz-adapter (#114)" (#115) This reverts commit c8146e0. * Revert "Add markdown landing page feature for authz-adapter (#114)" (#116) This reverts commit c8146e0. * Add simple static landing page with nginx and client-side markdown rendering (#117) * Initial plan * Add simple static landing page with nginx and client-side markdown rendering Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add SRI integrity check for marked.js and document hostPath security Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add DOMPurify for XSS protection when rendering markdown Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add security headers to nginx config and fix error message consistency Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Update README with security notes about CDN and hostPath Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add landing-page to docker-install target and make image configurable Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Document landingPage.docsPath configuration and content update behavior Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * Add documentation for patching kind cluster with extraMounts config Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * adds landing page mount * updates landing page content * updates landing page content --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> Co-authored-by: Brian Walsh <brian@bwalsh.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * adds image * WIP gitapp * doc events to respond to * adds git status ADR * deprecate image, add mermaid md * Refactor landing page to use Docker-built content instead of ConfigMap (#120) * Initial plan * Refactor landing page to use Docker-built content instead of ConfigMap Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com> * landing page version * tweak landing page * improve style * improve landing page * WIP write status back to github * github commit status --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: bwalsh <47808+bwalsh@users.noreply.github.com>
* authz * improve landing page * write status back to github * github commit status
Feature Request: Authz-Aware Ingress Overlay for Multi-Tenant UIs and APIs
Summary
Add a small Helm overlay to
helm/argo-stackthat:/workflows→argo-stack-argo-workflows-server(2746)/applications→argo-stack-argocd-server(8080)/registrations→github-repo-registrations-eventsource-svc(12000)/api→calypr-api(3000, namespacecalypr-api)/tenants→calypr-tenants(3001, namespacecalypr-tenants)authz-adapter(NGINX external auth)helm/argo-stack/overlays/ingress-authz-overlay