Repository navigation
support route exposure for Central endpoint #331
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
db4b0de
ebdddaf
fdf9064
b65ff3e
10005a8
c4d2fb9
8a87df5
d113e37
a7a4498
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |
| "context" | ||
| "crypto/x509" | ||
| "encoding/base64" | ||
| "encoding/json" | ||
| "encoding/pem" | ||
| "errors" | ||
| "fmt" | ||
|
|
@@ -686,12 +687,95 @@ func (d *Deployer) waitForLoadBalancer(ctx context.Context, namespace, serviceNa | |
| } | ||
| } | ||
|
|
||
| time.Sleep(5 * time.Second) | ||
| select { | ||
| case <-time.After(5 * time.Second): | ||
| case <-ctx.Done(): | ||
| return "", fmt.Errorf("cancelled waiting for LoadBalancer to get external address: %w", ctx.Err()) | ||
| } | ||
| } | ||
|
|
||
| return "", errors.New("timeout waiting for LoadBalancer to get external address") | ||
| } | ||
|
|
||
| // routeStatus is the subset of the OpenShift Route object waitForRoute needs. | ||
| type routeStatus struct { | ||
| Spec struct { | ||
| Host string `json:"host"` | ||
| } `json:"spec"` | ||
| Status struct { | ||
| Ingress []struct { | ||
| Host string `json:"host"` | ||
| Conditions []struct { | ||
| Type string `json:"type"` | ||
| Status string `json:"status"` | ||
| } `json:"conditions"` | ||
| } `json:"ingress"` | ||
| } `json:"status"` | ||
| } | ||
|
|
||
| // fetchRouteStatus reads the Route's host and admission state from a single | ||
| // object, so both reflect the same state of the Route. Admission conditions | ||
| // live on the per-router status.ingress entries; a host conflict can leave the | ||
| // Route unadmitted with a host set, so admission is only reported when an | ||
| // ingress entry for the host carries Admitted=True. | ||
| func fetchRouteStatus(ctx context.Context, namespace, routeName string) (host string, admitted bool, err error) { | ||
| result, err := k8s.RunKubectl(ctx, k8s.KubectlOptions{ | ||
| Args: []string{"get", "route", routeName, "-n", namespace, "-o", "json"}, | ||
| }) | ||
| if err != nil { | ||
| return "", false, err | ||
| } | ||
| var route routeStatus | ||
| if err := json.Unmarshal([]byte(result.Stdout), &route); err != nil { | ||
| return "", false, err | ||
| } | ||
| host, admitted = routeAdmittedFor(route) | ||
| return host, admitted, nil | ||
| } | ||
|
|
||
| // routeAdmittedFor reports the Route's host and whether an ingress entry for | ||
| // that host carries an Admitted=True condition. | ||
| func routeAdmittedFor(route routeStatus) (string, bool) { | ||
| for _, ingress := range route.Status.Ingress { | ||
| if ingress.Host != route.Spec.Host { | ||
| continue | ||
| } | ||
| for _, condition := range ingress.Conditions { | ||
| if condition.Type == "Admitted" && condition.Status == "True" { | ||
| return route.Spec.Host, true | ||
| } | ||
| } | ||
| } | ||
| return route.Spec.Host, false | ||
| } | ||
|
|
||
| // waitForRoute waits for an OpenShift Route to be admitted by the router. | ||
| // Returns the endpoint as "host:port", with no https:// prefix. | ||
| func (d *Deployer) waitForRoute(ctx context.Context, namespace, routeName string, waitTime time.Duration) (string, error) { | ||
| log.Infof("⏳ Waiting %s for Route %s to be admitted...", waitTime, routeName) | ||
|
|
||
| start := time.Now() | ||
| for time.Since(start) < waitTime { | ||
| hostname, admitted, err := fetchRouteStatus(ctx, namespace, routeName) | ||
| if err == nil && hostname != "" && admitted { | ||
| if env.RunningInteractively { | ||
| log.Successf("✓ Route admitted with hostname: %s", hostname) | ||
| } else { | ||
| log.Success("✓ Route admitted") | ||
| } | ||
| return fmt.Sprintf("%s:443", hostname), nil | ||
| } | ||
|
|
||
| select { | ||
| case <-time.After(5 * time.Second): | ||
| case <-ctx.Done(): | ||
| return "", fmt.Errorf("cancelled waiting for Route to be admitted: %w", ctx.Err()) | ||
| } | ||
| } | ||
|
|
||
| return "", errors.New("timeout waiting for Route to be admitted") | ||
| } | ||
|
|
||
| // fetchCentralCACerts fetches CA certificates needed to verify Central's TLS cert. | ||
| // It always fetches the internal StackRox CA from the central-tls secret, and | ||
| // additionally fetches CA certs from the defaultTLSSecret if one is configured | ||
|
|
@@ -832,6 +916,12 @@ func (d *Deployer) configureCentralEndpoint(ctx context.Context) error { | |
| return fmt.Errorf("failed to get LoadBalancer endpoint: %w", err) | ||
| } | ||
| d.centralEndpoint = endpoint | ||
| } else if exposure == types.ExposureRoute { | ||
| endpoint, err := d.waitForRoute(ctx, d.config.Central.Namespace, "central", loadBalancerWaitTime) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to get Route endpoint: %w", err) | ||
| } | ||
| d.centralEndpoint = endpoint | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } else { | ||
| d.centralEndpoint = "central." + d.config.Central.Namespace + ".svc:443" | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package deployer | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestRouteStatusParse(t *testing.T) { | ||
| raw := ` | ||
| { | ||
| "spec": { | ||
| "host": "central-acs-central.apps-crc.testing" | ||
| }, | ||
| "status": { | ||
| "ingress": [{ | ||
| "host": "central-acs-central.apps-crc.testing", | ||
| "conditions": [{ | ||
| "type": "Admitted", | ||
| "status": "True", | ||
| "lastTransitionTime": "x" | ||
| }] | ||
| }] | ||
| } | ||
| }` | ||
| var r routeStatus | ||
| if err := json.Unmarshal([]byte(raw), &r); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
Comment on lines
+26
to
+28
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about less control-flow, simplifying the test code? err := json.Unmarshal([]byte(raw), &r)
require.NoError(t, err) |
||
| if r.Spec.Host != "central-acs-central.apps-crc.testing" { | ||
| t.Fatalf("host: %q", r.Spec.Host) | ||
| } | ||
|
Comment on lines
+29
to
+31
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not the more concise version asssert.Equal(t, "central-acs-central.apps-crc.testing", r.Spec.Host) |
||
| host, admitted := routeAdmittedFor(r) | ||
| if !admitted || host != r.Spec.Host { | ||
| t.Fatalf("admitted=%v host=%q", admitted, host) | ||
| } | ||
|
Comment on lines
+33
to
+35
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar question here Same below. |
||
| // unadmitted / host-mismatch variants | ||
| raw2 := `{"spec":{"host":"h1"},"status":{"ingress":[{"host":"h2","conditions":[{"type":"Admitted","status":"True"}]}]}}` | ||
| json.Unmarshal([]byte(raw2), &r) | ||
| if _, admitted := routeAdmittedFor(r); admitted { | ||
| t.Fatal("host mismatch must not be admitted") | ||
| } | ||
| raw3 := `{"spec":{"host":"h1"},"status":{"ingress":[{"host":"h1","conditions":[{"type":"Admitted","status":"False"}]}]}}` | ||
| json.Unmarshal([]byte(raw3), &r) | ||
| if _, admitted := routeAdmittedFor(r); admitted { | ||
| t.Fatal("Admitted=False must not be admitted") | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to continue looping in case the inner for-loop (L743-747) does not find what it is looking for?
I think I would have a slight preference for encoding the happy path logic outside of these for loops, keeping the loops primarily as means for navigating the object structure, e.g.
This has the early exit built-in, but at the same time it might be slightly more verbose and less idiomatic Go?
If sticking with your more compact version, I'd suggest to add an early return adter L747?