Feat expose deployment ingress #297
Conversation
WalkthroughThis change introduces support for injecting ingress-related environment variables into Akash Provider deployment containers. It adds logic to collect all ingress URIs and the provider's ingress address, sets them as environment variables, documents the feature, and extends tests to verify correct variable injection. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Deployment YAML (SDL)
participant Workload Builder
participant K8s Container
User->>Deployment YAML (SDL): Defines exposes/ingresses
Deployment YAML (SDL)->>Workload Builder: Deployment spec parsed
Workload Builder->>Workload Builder: collectIngressURIs()
Workload Builder->>Workload Builder: Determine static/custom hosts, protocols, ports
Workload Builder->>Workload Builder: Compose ingress URIs
Workload Builder->>K8s Container: Inject AKASH_INGRESS_URIS, AKASH_PROVIDER_INGRESS env vars
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
cluster/kube/builder/deployment_test.go (2)
81-86: Variable name shadows imported package
TestDeploySetsEnvironmentVariablesdeclaressdl, err := sdl.ReadFile(...)shadowing the imported
github.com/akash-network/node/sdlpackage for the rest of the function.
While it compiles, it is easy to mis-read and prevents future package references.- sdl, err := sdl.ReadFile(...) + sdlBytes, err := sdl.ReadFile(...)Same issue exists in the new test below – please rename both locals.
88-142: Strengthen new ingress test & reduce duplicationThe new test duplicates ~40 lines of setup already present above. Extract a small
helper that returnsenv map[string]stringto keep tests concise.Additionally, the purpose of this test is to validate
AKASH_INGRESS_URIS, yet
it only assertsAKASH_PROVIDER_INGRESS. Consider asserting that the variable
exists and is non-empty (or matches an expected value derived from the test
fixture). Otherwise the test may pass even if URI collection silently breaks._docs/ingress-environment-variables.md (1)
180-182: Grammar nit – hyphenate compound adjective“Fully backward compatible” → “Fully backward-compatible”.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~181-~181: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... providers Compatibility: - Fully backward compatible - Optional usage - applications can ign...(EN_COMPOUND_ADJECTIVE_INTERNAL)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
_docs/ingress-environment-variables.md(1 hunks)cluster/kube/builder/builder.go(1 hunks)cluster/kube/builder/deployment_test.go(1 hunks)cluster/kube/builder/workload.go(3 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
cluster/kube/builder/workload.go
423-423: string https has 3 occurrences, make it a constant
(goconst)
421-421: string http has 3 occurrences, make it a constant
(goconst)
🪛 LanguageTool
_docs/ingress-environment-variables.md
[uncategorized] ~181-~181: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... providers Compatibility: - Fully backward compatible - Optional usage - applications can ign...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🔇 Additional comments (2)
cluster/kube/builder/builder.go (1)
60-62: New env-var constants fit existing naming scheme – LGTMThe additions are self-explanatory and keep the
AKASH_*convention consistent.
No concerns here.cluster/kube/builder/workload.go (1)
467-477: Minor: skip env insertion when hostname emptyThe guard already checks
b.settings.ClusterPublicHostname != "", so the call to
addIfNotPresentnever receives an empty value. Good.(Just noting this because the earlier block adds
AKASH_CLUSTER_PUBLIC_HOSTNAMEunconditionally – the asymmetry is intentional and fine.)
roylee17
force-pushed
the
feat-expose-deployment-ingress
branch
from
00b2c90 to
aa7ffe4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
cluster/kube/builder/workload_test.go (3)
12-18: Run tests concurrently witht.Parallel()All test functions in this file are independent and have no shared state. Calling
t.Parallel()at the top of each test speeds up the suite and is idiomatic in Go.func TestCollectIngressURIsDeduplication(t *testing.T) { - log := testutil.Logger(t) + t.Parallel() + log := testutil.Logger(t)(Apply the same one-liner to the other test functions.)
66-75: Simplify duplicate-detection assertionCreating a map and looping manually works, but
require.ElementsMatch(orassert.ElementsMatch) is clearer and less error-prone for set comparisons.-// Basic test - ensure no duplicates exist -uriSet := make(map[string]bool) -for _, uri := range uris { - require.False(t, uriSet[uri], "Found duplicate URI: %s", uri) - uriSet[uri] = true -} - -// Test passes if we have some URIs and no duplicates -require.Greater(t, len(uris), 0, "collectIngressURIs should return URIs for the configured ingress") +require.Greater(t, len(uris), 0, "collectIngressURIs should return at least one URI") +require.ElementsMatch(t, uris, uris, "collectIngressURIs returned duplicates") // ElementsMatch de-dupes internally
111-136: Constant re-evaluation duplicates earlier test
TestProtocolConstantsUsagelargely re-executes logic already covered byTestProtocolConstantsandTestDeduplicationLogic. If the intent is to unit-testcollectIngressURIs, consider turning this into a table-driven test of that function directly to avoid redundant coverage.Not blocking, but pruning duplicated tests keeps the suite lean.
_docs/ingress-environment-variables.md (1)
179-183: Hyphenate compound adjective“Backward-compatible” should be hyphenated when used as a compound adjective.
- - Fully backward compatible + - Fully backward-compatible🧰 Tools
🪛 LanguageTool
[uncategorized] ~181-~181: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... providers Compatibility: - Fully backward compatible - Optional usage - applications can ign...(EN_COMPOUND_ADJECTIVE_INTERNAL)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
_docs/ingress-environment-variables.md(1 hunks)cluster/kube/builder/builder.go(1 hunks)cluster/kube/builder/deployment_test.go(1 hunks)cluster/kube/builder/workload.go(4 hunks)cluster/kube/builder/workload_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- cluster/kube/builder/deployment_test.go
- cluster/kube/builder/builder.go
- cluster/kube/builder/workload.go
🧰 Additional context used
🪛 LanguageTool
_docs/ingress-environment-variables.md
[uncategorized] ~181-~181: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... providers Compatibility: - Fully backward compatible - Optional usage - applications can ign...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🔇 Additional comments (1)
cluster/kube/builder/workload_test.go (1)
86-109: Unit test doesn’t reflect real-world normalisation
collectIngressURIsomits default ports (80/443). In this synthetic test the URIhttps://example.com:443would never be produced by the production code, so the scenario is not representative and risks future confusion.Consider changing the sample data to match the real normalisation (
https://example.com) or explicitly documenting why the port is kept here.
roylee17
force-pushed
the
feat-expose-deployment-ingress
branch
from
aa7ffe4 to
22e6c44
Compare
This change adds two new environment variables that are automatically injected into all containers in a deployment: 1. AKASH_INGRESS_URIS: Comma-separated list of all ingress URIs for the deployment, including both provider-generated static hosts and custom hosts specified in the SDL 2. AKASH_PROVIDER_INGRESS: The provider's public hostname/IP address that can be used for nodePort services These environment variables enable applications to: - Discover their own ingress endpoints programmatically - Build correct URLs for inter-service communication - Configure callback URLs and webhooks dynamically - Implement health checks and service discovery The implementation automatically detects ingress-capable services, handles both HTTP and HTTPS protocols, and properly formats URLs with or without non-standard ports.
roylee17
force-pushed
the
feat-expose-deployment-ingress
branch
from
22e6c44 to
9ce59a4
Compare
|
see #273. I basically already did about half of this? though the more important part, IMO, is the node ports which are more involved and require a init script. |
Add ingress environment variables to containers
Fixes akash-network/support#44
Summary
This PR adds two new environment variables to deployment containers to expose ingress information, enabling applications to discover their externally accessible URLs and provider ingress addresses.
Changes
Environment Variables Added
AKASH_INGRESS_URIS: Comma-separated list of all ingress URIs for the deploymentAKASH_PROVIDER_INGRESS: Provider's ingress address for nodePort servicesImplementation Details
cluster/kube/builder/workload.gocollectIngressURIs()- Gathers all ingress-capable service exposuresaddEnvVarsForDeployment()functionFeatures
Comprehensive URI Collection: Handles both provider-generated static hosts and custom SDL-specified hosts
Protocol Detection: Automatically detects HTTP/HTTPS based on port 443
Port Handling: Correctly formats URIs with non-standard ports
Zero Runtime Overhead: Variables computed at deployment creation time
Backwards Compatible: No breaking changes to existing functionality
Example Usage
For a deployment with ingress services, containers will receive: