Add metaldata#941
Open
maxmoehl wants to merge 1 commit into
Open
Conversation
Resolves: #932 Signed-off-by: Maximilian Moehl <maximilian@moehl.eu>
github-actions
Bot
added
size/XXL
api-change
documentation
Contributor
📝 WalkthroughWalkthroughThis PR introduces a complete metadata service ( ChangesMetaldata Service Implementation
Sequence Diagram(s)sequenceDiagram
participant Client
participant Flavor as Metadata-Flavor<br/>Middleware
participant Handler as v1 Handler
participant Index
participant Manager as Manager<br/>(cache)
participant APIReader as API Reader
Client->>Flavor: GET /v1/rack + Header
Flavor->>Flavor: validate Metadata-Flavor: IronCore Metal
alt Header missing/invalid
Flavor->>Client: 403 Forbidden
else Valid
Flavor->>Handler: proceed
Handler->>Handler: parse client IP
Handler->>Index: Lookup(IP)
Index-->>Handler: ServerEntry (name, metadata, claimRef)
Handler->>Handler: build response metadata
alt user-data requested
Handler->>Manager: Get ServerClaim by claimRef
Manager-->>Handler: ServerClaim or NotFound
alt claim has UserDataRef
Handler->>APIReader: Get Secret by UserDataRef
APIReader-->>Handler: Secret or NotFound
alt Secret type == user-data
Handler->>Handler: convert bytes to strings
Handler->>Client: 200 JSON (metadata + user-data)
else Wrong type
Handler->>Client: 404 Not Found
end
else No UserDataRef
Handler->>Client: 404 Not Found
end
else Metadata-only request
Handler->>Client: 200 JSON (metadata)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
internal/metaldata/index_test.go (1)
116-176: ⚡ Quick winAdd a regression test for duplicate IP ownership transitions.
Please add a case where Server A and Server B share an IP, then A is deleted/updated; lookup should still return B. This prevents silent regressions in conflict handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/metaldata/index_test.go` around lines 116 - 176, Add a regression test that verifies duplicate IP ownership transitions: create Server A and Server B that both include the same IP (use newServer/addServer to add both), then delete or update Server A (use deleteServer or updateServer) and assert that idx.Lookup(theSharedIP) still returns an entry and that entry corresponds to Server B; use idx.Lookup, addServer, deleteServer, updateServer, and newServer to locate the test scaffolding and ensure the shared-IP case is exercised for both delete and update transitions so the index does not drop B's ownership when A is removed.internal/metaldata/http.go (1)
1-280: Reminder: Runmake lint-fixandmake testafter editing.As per coding guidelines, Go source files should be validated with
make lint-fixandmake testafter changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/metaldata/http.go` around lines 1 - 280, The review is a reminder to validate changes with linters and tests; run make lint-fix and make test and address any issues they report for the modified symbols in this file (e.g. Server.Start, NewServer, accessLogger, metadataFlavorMiddleware, and the v1Handler methods like serveFullMetadata/serveField/serveUserData/serveUserDataKey) until both lint and test pass. Ensure any reported formatting/go vet/staticcheck errors are fixed, unresolved nil/err handling or unused imports are corrected, and then re-run make lint-fix and make test to confirm the file is clean.Source: Coding guidelines
internal/metaldata/http_test.go (1)
1-294: Reminder: Runmake testafter editing.As per coding guidelines, Go source files should be validated with
make testafter changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/metaldata/http_test.go` around lines 1 - 294, The test suite must be run and any failing tests fixed before merging: run `make test` and address failures related to the HTTP metadata tests by inspecting and correcting the test helpers and test logic (functions getMetadata, decodeJSON, readBody, claim, userDataSecret and the Describe blocks for metadata-flavor, /v1/, /v1/{field}, /v1/user-data and /v1/user-data/{key}) so that all expectations pass; iterate until `make test` completes successfully and then commit the green test results.Source: Coding guidelines
cmd/metaldata/main.go (2)
1-154: Reminder: Runmake lint-fixandmake testafter editing.As per coding guidelines, Go source files should be validated with
make lint-fixandmake testafter changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/metaldata/main.go` around lines 1 - 154, The PR needs linting and tests run; run "make lint-fix" and "make test", fix any issues tools report (formatting/gofmt, goimports, vet/lint warnings, and failing tests) and commit the fixes; specifically run these against the edited symbols Main, main, stringMapFlag.Set and stringMapFlag.String (and any changes in cmd/metaldata/main.go) and address whatever the linters/tests point to (e.g., unused imports, formatting, error handling or test failures) until both commands succeed.Source: Coding guidelines
87-93: ⚡ Quick winMake development mode configurable for production deployments.
The
Development: truesetting is hardcoded in the zap logger options. Development mode uses console encoding and adds stack traces, which are typically undesirable in production environments where structured JSON logging is preferred for log aggregation systems.Consider adding a flag to control this behavior.
♻️ Proposed change to add development mode flag
func Main() int { var ( listenPort int metricsBindAddress string healthBindAddress string staticMetadata stringMapFlag + development bool ) flag.IntVar(&listenPort, "metaldata-port", 10002, "The port the metadata HTTP server listens on.") flag.StringVar(&metricsBindAddress, "metrics-bind-address", ":8080", "The address the metrics endpoint binds to. Use \"0\" to disable.") flag.StringVar(&healthBindAddress, "health-probe-bind-address", ":8081", "The address the health probe endpoint binds to. Use \"0\" to disable.") flag.Var(&staticMetadata, "static-metadata", "Static metadata key=value pair exposed to all servers. May be repeated.") + flag.BoolVar(&development, "development", false, + "Enable development mode for the logger (console encoding, stack traces).") opts := zap.Options{ - Development: true, + Development: development, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/metaldata/main.go` around lines 87 - 93, The zap.Options currently hardcodes Development: true; make this configurable by adding a boolean CLI flag (e.g., dev or development) and use its value to set opts.Development after flag.Parse. Locate the zap.Options struct named opts and the BindFlags call on flag.CommandLine, add a flag definition before flag.Parse, then set opts.Development = <flagValue> (or bind the flag into opts via BindFlags) and keep the existing ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) so production can default to false and dev can be enabled via the flag.config/metaldata/daemonset.yaml (1)
54-65: ⚡ Quick winConsider hardening the container security context.
The container security context correctly drops all capabilities and disables privilege escalation, but it is missing
readOnlyRootFilesystem: true. Since metaldata is a stateless HTTP service that reads from the Kubernetes API and serves responses from an in-memory index, it does not require a writable filesystem.🛡️ Recommended security hardening
securityContext: allowPrivilegeEscalation: false + readOnlyRootFilesystem: true capabilities: drop: - "ALL"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config/metaldata/daemonset.yaml` around lines 54 - 65, Add readOnlyRootFilesystem: true to the container's securityContext in the DaemonSet spec so the container runs with a read-only root filesystem; update the existing securityContext block that contains allowPrivilegeEscalation and capabilities.drop (the same block shown) to include readOnlyRootFilesystem: true, ensuring it sits alongside allowPrivilegeEscalation and capabilities to harden the container for this stateless metaldata HTTP service.internal/metaldata/metaldata_suite_test.go (2)
71-75: 💤 Low valueConsider improving error propagation for faster failure detection.
If
srv.Start(ctx)fails immediately (e.g., port binding error), theEventuallyblock will time out before the goroutine'sExpectfailure is reported, making debugging harder.💡 Optional pattern for faster failure detection
+startedCh := make(chan struct{}) +errCh := make(chan error, 1) go func() { defer GinkgoRecover() - Expect(srv.Start(ctx)).To(Succeed(), "failed to start metaldata server") + close(startedCh) // Signal that goroutine started + if err := srv.Start(ctx); err != nil { + errCh <- err + } }() -Eventually(func() error { - _, err := http.Get(testServerURL) - return err -}).Should(Succeed()) +Eventually(func() error { + select { + case err := <-errCh: + return fmt.Errorf("server failed to start: %w", err) + default: + } + _, err := http.Get(testServerURL) + return err +}).Should(Succeed())This allows early detection of startup failures rather than waiting for the full timeout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/metaldata/metaldata_suite_test.go` around lines 71 - 75, The goroutine-start pattern around srv.Start(ctx) can hide immediate startup errors (like bind failures) until Eventually times out; modify the test to surface startup errors quickly by running srv.Start(ctx) inside the goroutine but sending its returned error (or nil) on a channel (e.g., errCh) and then immediately select on that channel with a short timeout before proceeding to the Eventually checks; update references to srv.Start, the anonymous goroutine, and the Expect call to read from errCh and call Expect(errChVal).To(Succeed()) so startup failures are asserted fast.
77-80: ⚡ Quick winConsider adding an explicit timeout and polling a valid endpoint.
The
Eventuallyblock lacks an explicit timeout and polls the root path/which returns 404 (handlers are at/v1/...). While this works for checking server availability (connection success regardless of status code), it could be more explicit.🔧 Suggested improvements
Eventually(func() error { - _, err := http.Get(testServerURL) + resp, err := http.Get(testServerURL + "/v1/") + if err != nil { + return err + } + resp.Body.Close() + if resp.StatusCode >= 500 { + return fmt.Errorf("server error: %d", resp.StatusCode) + } return err -}).Should(Succeed()) +}, "10s", "100ms").Should(Succeed())Changes:
- Poll
/v1/instead of/to verify actual handler registration- Add explicit 10-second timeout and 100ms polling interval
- Check for 5xx errors (server not ready) vs 4xx (expected for empty request)
- Close response body to avoid resource leak
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/metaldata/metaldata_suite_test.go` around lines 77 - 80, The Eventually HTTP probe should target a valid handler and include timeout/polling and response-body cleanup: change the probe to call http.Get on testServerURL + "/v1/" (or the appropriate registered path), add an explicit timeout of 10s and a 100ms polling interval to the Eventually call, inspect the returned *http.Response to close resp.Body to avoid leaks and treat 5xx as retryable errors while considering 4xx (e.g., 404/400) as acceptable success for handler presence; update the block around Eventually, http.Get and response handling accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/metaldata/http.go`:
- Around line 79-82: The http.Server instantiation for srv currently omits
timeout settings, leaving the service vulnerable to slow-client/resource
exhaustion; update the srv: &http.Server{ ... } block (where Addr, Handler are
set and accessLogger/mux are used) to include sensible ReadHeaderTimeout,
WriteTimeout, and IdleTimeout values (e.g., seconds-level durations) to cap how
long headers, responses, and idle connections can persist; ensure durations are
set via time.Second or time.Duration constants and choose values appropriate for
metadata request latency.
In `@internal/metaldata/index.go`:
- Around line 145-147: When removing addresses from the hosts map, only delete
an entry if the current owner still matches the server being removed to avoid
wiping a recently reassigned IP; for each addr in addrs, look up idx.hosts[addr]
and compare it to the identifier of the server being removed (the same ID used
when that server registered), and call delete(idx.hosts, addr) only when they
are equal. Ensure you reference the existing symbols idx.hosts and addrs and use
the server ID variable used in this function (e.g., the removal target’s ID) for
the comparison so concurrent reassignments aren’t lost.
In `@internal/metaldata/metaldata_suite_test.go`:
- Around line 24-25: Tests use a hardcoded port (30003) causing flakiness;
modify the suite to request port 0 from metaldata.NewServer so the OS assigns a
free port, start the server (via srv.Start or similar), then expose and read the
actual bound port from the server (add a method like Server.Port() or similar on
the metaldata.Server type that returns the bound port after Start/Listen begins)
and build testServerURL dynamically with fmt.Sprintf("http://127.0.0.1:%d",
srv.Port()); ensure Start is launched in a goroutine with GinkgoRecover and the
test waits for the server to be ready before using testServerURL.
---
Nitpick comments:
In `@cmd/metaldata/main.go`:
- Around line 1-154: The PR needs linting and tests run; run "make lint-fix" and
"make test", fix any issues tools report (formatting/gofmt, goimports, vet/lint
warnings, and failing tests) and commit the fixes; specifically run these
against the edited symbols Main, main, stringMapFlag.Set and
stringMapFlag.String (and any changes in cmd/metaldata/main.go) and address
whatever the linters/tests point to (e.g., unused imports, formatting, error
handling or test failures) until both commands succeed.
- Around line 87-93: The zap.Options currently hardcodes Development: true; make
this configurable by adding a boolean CLI flag (e.g., dev or development) and
use its value to set opts.Development after flag.Parse. Locate the zap.Options
struct named opts and the BindFlags call on flag.CommandLine, add a flag
definition before flag.Parse, then set opts.Development = <flagValue> (or bind
the flag into opts via BindFlags) and keep the existing
ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) so production can default to
false and dev can be enabled via the flag.
In `@config/metaldata/daemonset.yaml`:
- Around line 54-65: Add readOnlyRootFilesystem: true to the container's
securityContext in the DaemonSet spec so the container runs with a read-only
root filesystem; update the existing securityContext block that contains
allowPrivilegeEscalation and capabilities.drop (the same block shown) to include
readOnlyRootFilesystem: true, ensuring it sits alongside
allowPrivilegeEscalation and capabilities to harden the container for this
stateless metaldata HTTP service.
In `@internal/metaldata/http_test.go`:
- Around line 1-294: The test suite must be run and any failing tests fixed
before merging: run `make test` and address failures related to the HTTP
metadata tests by inspecting and correcting the test helpers and test logic
(functions getMetadata, decodeJSON, readBody, claim, userDataSecret and the
Describe blocks for metadata-flavor, /v1/, /v1/{field}, /v1/user-data and
/v1/user-data/{key}) so that all expectations pass; iterate until `make test`
completes successfully and then commit the green test results.
In `@internal/metaldata/http.go`:
- Around line 1-280: The review is a reminder to validate changes with linters
and tests; run make lint-fix and make test and address any issues they report
for the modified symbols in this file (e.g. Server.Start, NewServer,
accessLogger, metadataFlavorMiddleware, and the v1Handler methods like
serveFullMetadata/serveField/serveUserData/serveUserDataKey) until both lint and
test pass. Ensure any reported formatting/go vet/staticcheck errors are fixed,
unresolved nil/err handling or unused imports are corrected, and then re-run
make lint-fix and make test to confirm the file is clean.
In `@internal/metaldata/index_test.go`:
- Around line 116-176: Add a regression test that verifies duplicate IP
ownership transitions: create Server A and Server B that both include the same
IP (use newServer/addServer to add both), then delete or update Server A (use
deleteServer or updateServer) and assert that idx.Lookup(theSharedIP) still
returns an entry and that entry corresponds to Server B; use idx.Lookup,
addServer, deleteServer, updateServer, and newServer to locate the test
scaffolding and ensure the shared-IP case is exercised for both delete and
update transitions so the index does not drop B's ownership when A is removed.
In `@internal/metaldata/metaldata_suite_test.go`:
- Around line 71-75: The goroutine-start pattern around srv.Start(ctx) can hide
immediate startup errors (like bind failures) until Eventually times out; modify
the test to surface startup errors quickly by running srv.Start(ctx) inside the
goroutine but sending its returned error (or nil) on a channel (e.g., errCh) and
then immediately select on that channel with a short timeout before proceeding
to the Eventually checks; update references to srv.Start, the anonymous
goroutine, and the Expect call to read from errCh and call
Expect(errChVal).To(Succeed()) so startup failures are asserted fast.
- Around line 77-80: The Eventually HTTP probe should target a valid handler and
include timeout/polling and response-body cleanup: change the probe to call
http.Get on testServerURL + "/v1/" (or the appropriate registered path), add an
explicit timeout of 10s and a 100ms polling interval to the Eventually call,
inspect the returned *http.Response to close resp.Body to avoid leaks and treat
5xx as retryable errors while considering 4xx (e.g., 404/400) as acceptable
success for handler presence; update the block around Eventually, http.Get and
response handling accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fd93f171-d195-480e-808f-1ea272d95020
⛔ Files ignored due to path filters (1)
dist/chart/templates/crd/metal.ironcore.dev_serverclaims.yamlis excluded by!**/dist/**
📒 Files selected for processing (23)
DockerfileMakefileapi/v1alpha1/applyconfiguration/api/v1alpha1/serverclaimspec.goapi/v1alpha1/applyconfiguration/internal/internal.goapi/v1alpha1/constants.goapi/v1alpha1/server_types.goapi/v1alpha1/serverclaim_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/metaldata/main.goconfig/crd/bases/metal.ironcore.dev_serverclaims.yamlconfig/default/kustomization.yamlconfig/metaldata/daemonset.yamlconfig/metaldata/kustomization.yamlconfig/metaldata/role.yamlconfig/metaldata/role_binding.yamlconfig/metaldata/service.yamlconfig/metaldata/service_account.yamldocs/api-reference/api.mdinternal/metaldata/http.gointernal/metaldata/http_test.gointernal/metaldata/index.gointernal/metaldata/index_test.gointernal/metaldata/metaldata_suite_test.go
Comment on lines
+79
to
+82
| srv: &http.Server{ | ||
| Addr: net.JoinHostPort("", strconv.Itoa(port)), | ||
| Handler: accessLogger(log.WithName("access"), mux), | ||
| }, |
Contributor
There was a problem hiding this comment.
Configure server timeouts to prevent resource exhaustion.
The http.Server lacks timeout configurations (ReadHeaderTimeout, WriteTimeout, IdleTimeout). Without ReadHeaderTimeout, the server is vulnerable to Slowloris-style attacks where clients hold connections open by slowly sending headers. For a service handling metadata requests from potentially many bare-metal servers, these timeouts are essential for stability.
🛡️ Recommended timeout configuration
return &Server{
srv: &http.Server{
Addr: net.JoinHostPort("", strconv.Itoa(port)),
Handler: accessLogger(log.WithName("access"), mux),
+ ReadHeaderTimeout: 10 * time.Second,
+ WriteTimeout: 30 * time.Second,
+ IdleTimeout: 90 * time.Second,
},
log: log,
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| srv: &http.Server{ | |
| Addr: net.JoinHostPort("", strconv.Itoa(port)), | |
| Handler: accessLogger(log.WithName("access"), mux), | |
| }, | |
| srv: &http.Server{ | |
| Addr: net.JoinHostPort("", strconv.Itoa(port)), | |
| Handler: accessLogger(log.WithName("access"), mux), | |
| ReadHeaderTimeout: 10 * time.Second, | |
| WriteTimeout: 30 * time.Second, | |
| IdleTimeout: 90 * time.Second, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/metaldata/http.go` around lines 79 - 82, The http.Server
instantiation for srv currently omits timeout settings, leaving the service
vulnerable to slow-client/resource exhaustion; update the srv: &http.Server{ ...
} block (where Addr, Handler are set and accessLogger/mux are used) to include
sensible ReadHeaderTimeout, WriteTimeout, and IdleTimeout values (e.g.,
seconds-level durations) to cap how long headers, responses, and idle
connections can persist; ensure durations are set via time.Second or
time.Duration constants and choose values appropriate for metadata request
latency.
Comment on lines
+145
to
+147
| for _, addr := range addrs { | ||
| delete(idx.hosts, addr) | ||
| } |
Contributor
There was a problem hiding this comment.
Guard IP removal by current owner to avoid deleting another Server’s mapping.
Line 145-147 deletes hosts[addr] unconditionally. After Line 127-132 overwrites an IP to a different server, deleting the old server will remove the new server’s live entry.
Proposed fix
func (idx *Index) removeLocked(name string) {
addrs, ok := idx.serverIPs[name]
if !ok {
return
}
for _, addr := range addrs {
- delete(idx.hosts, addr)
+ if entry, exists := idx.hosts[addr]; exists && entry.Name == name {
+ delete(idx.hosts, addr)
+ }
}
delete(idx.serverIPs, name)
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/metaldata/index.go` around lines 145 - 147, When removing addresses
from the hosts map, only delete an entry if the current owner still matches the
server being removed to avoid wiping a recently reassigned IP; for each addr in
addrs, look up idx.hosts[addr] and compare it to the identifier of the server
being removed (the same ID used when that server registered), and call
delete(idx.hosts, addr) only when they are equal. Ensure you reference the
existing symbols idx.hosts and addrs and use the server ID variable used in this
function (e.g., the removal target’s ID) for the comparison so concurrent
reassignments aren’t lost.
Comment on lines
+24
to
+25
| testServerURL = "http://127.0.0.1:30003" | ||
| testListenPort = 30003 |
Contributor
There was a problem hiding this comment.
Use dynamic port allocation instead of hardcoded port 30003.
Hardcoded ports cause test flakiness from port conflicts and prevent parallel test execution. If port 30003 is already in use (by another test suite, service, or process), the suite will fail intermittently.
🔧 Recommended approach using OS-assigned port
- Pass
0as the port tometaldata.NewServerto let the OS assign a free port - After starting the server, retrieve the actual port from the listener
- Construct
testServerURLdynamically
Example pattern:
// Let OS assign a free port
srv := metaldata.NewServer(GinkgoLogr, idx, reader, 0)
go func() {
defer GinkgoRecover()
Expect(srv.Start(ctx)).To(Succeed())
}()
// Get actual assigned port (requires exposing listener or port from Server)
actualPort := srv.Port() // Would need to add this method to Server
testServerURL := fmt.Sprintf("http://127.0.0.1:%d", actualPort)Note: This requires the metaldata.Server type to expose its assigned port. If that's not currently available, consider adding a Port() int method that returns the actual bound port after ListenAndServe starts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/metaldata/metaldata_suite_test.go` around lines 24 - 25, Tests use a
hardcoded port (30003) causing flakiness; modify the suite to request port 0
from metaldata.NewServer so the OS assigns a free port, start the server (via
srv.Start or similar), then expose and read the actual bound port from the
server (add a method like Server.Port() or similar on the metaldata.Server type
that returns the bound port after Start/Listen begins) and build testServerURL
dynamically with fmt.Sprintf("http://127.0.0.1:%d", srv.Port()); ensure Start is
launched in a goroutine with GinkgoRecover and the test waits for the server to
be ready before using testServerURL.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves: #932
Summary by CodeRabbit
New Features
userDataReffield to ServerClaim allowing users to reference secrets containing user-data for claimed serversInfrastructure
Documentation