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.

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

enable noctx linter and fix errors #684

Merged
merged 1 commit into from
Aug 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ linters:
- nestif
- nilerr
- nilnil
# - noctx
- noctx
- nolintlint
- nonamedreturns
- nosprintfhostport
Expand Down
11 changes: 10 additions & 1 deletion cmd/cl-controlplane/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,19 @@ func (o *Options) Run() error {
readinessListenAddress := fmt.Sprintf("0.0.0.0:%d", api.ReadinessListenPort)
httpServer := utilhttp.NewServer("controlplane-http", nil)
httpServer.Router().Get("/", func(w http.ResponseWriter, r *http.Request) {
resp, err := http.Get(fmt.Sprintf("https://127.0.0.1:%d", api.ListenPort))
ctx := r.Context()
req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("https://127.0.0.1:%d", api.ListenPort), http.NoBody)
if err != nil {
logrus.Errorf("Error creating request: %v", err)
w.WriteHeader(http.StatusInternalServerError)
return
}

resp, err := http.DefaultClient.Do(req)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I would prefer not to use default client (e.g., it has no timeouts), but this can be done separately in a different PR

if err == nil && resp.Body.Close() != nil {
logrus.Infof("Cannot close readiness response body: %v", err)
}

if errors.Is(err, syscall.ECONNREFUSED) ||
errors.Is(err, syscall.ECONNRESET) ||
!authzManager.IsReady() ||
Expand Down
10 changes: 9 additions & 1 deletion cmd/cl-dataplane/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,15 @@ func (o *Options) Run() error {
return fmt.Errorf("cannot listen for readiness: %w", err)
}
httpServer.Router().Get("/", func(w http.ResponseWriter, r *http.Request) {
resp, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/ready", adminPort))
ctx := r.Context()
req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("http://127.0.0.1:%d/ready", adminPort), http.NoBody)
if err != nil {
logrus.Errorf("Error creating request: %v", err)
w.WriteHeader(http.StatusInternalServerError)
return
}

resp, err := http.DefaultClient.Do(req)
if err == nil && resp.Body.Close() != nil {
logrus.Infof("Cannot close readiness response body: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/cl-go-dataplane/app/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (o *Options) runGoDataplane(dataplaneID string, parsedCertData *tls.ParsedC
return fmt.Errorf("cannot listen for readiness: %w", err)
}
httpServer.Router().Get("/", func(w http.ResponseWriter, r *http.Request) {
if !xdsClient.IsReady() || !dataplane.IsReady() {
if !xdsClient.IsReady() || !dataplane.IsReady(r.Context()) {
w.WriteHeader(http.StatusServiceUnavailable)
}
})
Expand Down
12 changes: 7 additions & 5 deletions pkg/controlplane/authz/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,11 +324,13 @@ func (m *Manager) authorizeEgress(ctx context.Context, req *egressAuthorizationR
}

m.logger.Infof("Egress authorized. Sending authorization request to %s", importSource.Peer)
accessToken, err := cl.Authorize(&cpapi.AuthorizationRequest{
ServiceName: DstName,
ServiceNamespace: DstNamespace,
SrcAttributes: srcAttributes,
})
accessToken, err := cl.Authorize(
ctx,
&cpapi.AuthorizationRequest{
ServiceName: DstName,
ServiceNamespace: DstNamespace,
SrcAttributes: srcAttributes,
})
if err != nil {
m.logger.Infof("Unable to get access token from peer: %v", err)
continue
Expand Down
7 changes: 4 additions & 3 deletions pkg/controlplane/peer/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package peer

import (
"context"
"crypto/tls"
"encoding/json"
"errors"
Expand Down Expand Up @@ -94,14 +95,14 @@ func (c *Client) getResponse(
}

// Authorize a request for accessing a peer exported service, yielding an access token.
func (c *Client) Authorize(req *api.AuthorizationRequest) (string, error) {
func (c *Client) Authorize(ctx context.Context, req *api.AuthorizationRequest) (string, error) {
body, err := json.Marshal(req)
if err != nil {
return "", fmt.Errorf("unable to serialize authorization request: %w", err)
}

serverResp, err := c.getResponse(func(client *jsonapi.Client) (*jsonapi.Response, error) {
return client.Post(api.RemotePeerAuthorizationPath, body)
return client.Post(ctx, api.RemotePeerAuthorizationPath, body)
})
if err != nil {
return "", err
Expand All @@ -118,7 +119,7 @@ func (c *Client) Authorize(req *api.AuthorizationRequest) (string, error) {
// GetHeartbeat get a heartbeat from other peers.
func (c *Client) GetHeartbeat() error {
serverResp, err := c.getResponse(func(client *jsonapi.Client) (*jsonapi.Response, error) {
return client.Get(api.HeartbeatPath)
return client.Get(context.Background(), api.HeartbeatPath)
})
if err != nil {
return err
Expand Down
15 changes: 12 additions & 3 deletions pkg/dataplane/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package server

import (
"context"
"crypto/tls"
"errors"
"fmt"
Expand Down Expand Up @@ -60,8 +61,14 @@ func (d *Dataplane) StartDataplaneServer(dataplaneServerAddress string) error {
return server.ListenAndServeTLS("", "")
}

func (d *Dataplane) IsReady() bool {
resp, err := http.Get(fmt.Sprintf("https://127.0.0.1:%d", api.ListenPort))
func (d *Dataplane) IsReady(ctx context.Context) bool {
req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("https://127.0.0.1:%d", api.ListenPort), http.NoBody)
if err != nil {
d.logger.Errorf("Error creating readiness request: %v", err)
return false
}

resp, err := http.DefaultClient.Do(req)
if err == nil && resp.Body.Close() != nil {
d.logger.Infof("Cannot close readiness response body: %v", err)
}
Expand Down Expand Up @@ -239,14 +246,16 @@ func (d *Dataplane) initiateEgressConnection(targetCluster, authToken string, ap
return err
}

ctx := context.Background()

client := &http.Client{
Transport: &http.Transport{
TLSClientConfig: tlsConfig,
DialTLS: connDialer{peerConn}.Dial,
},
}

egressReq, err := http.NewRequest(http.MethodConnect, url, http.NoBody)
egressReq, err := http.NewRequestWithContext(ctx, http.MethodConnect, url, http.NoBody)
if err != nil {
return err
}
Expand Down
21 changes: 11 additions & 10 deletions pkg/util/jsonapi/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package jsonapi

import (
"bytes"
"context"
"crypto/tls"
"errors"
"fmt"
Expand Down Expand Up @@ -44,37 +45,37 @@ type Response struct {
}

// Get sends an HTTP GET request.
func (c *Client) Get(path string) (*Response, error) {
return c.do(http.MethodGet, path, nil)
func (c *Client) Get(ctx context.Context, path string) (*Response, error) {
return c.do(ctx, http.MethodGet, path, nil)
}

// Post sends an HTTP POST request.
func (c *Client) Post(path string, body []byte) (*Response, error) {
return c.do(http.MethodPost, path, body)
func (c *Client) Post(ctx context.Context, path string, body []byte) (*Response, error) {
return c.do(ctx, http.MethodPost, path, body)
}

// Put sends an HTTP PUT request.
func (c *Client) Put(path string, body []byte) (*Response, error) {
return c.do(http.MethodPut, path, body)
func (c *Client) Put(ctx context.Context, path string, body []byte) (*Response, error) {
return c.do(ctx, http.MethodPut, path, body)
}

// Delete sends an HTTP DELETE request.
func (c *Client) Delete(path string, body []byte) (*Response, error) {
return c.do(http.MethodDelete, path, body)
func (c *Client) Delete(ctx context.Context, path string, body []byte) (*Response, error) {
return c.do(ctx, http.MethodDelete, path, body)
}

// ServerURL returns the server URL configured for this client.
func (c *Client) ServerURL() string {
return c.serverURL
}

func (c *Client) do(method, path string, body []byte) (*Response, error) {
func (c *Client) do(ctx context.Context, method, path string, body []byte) (*Response, error) {
requestLogger := c.logger.WithFields(logrus.Fields{"method": method, "path": path})

requestLogger.WithField("body-length", len(body)).Debugf("Issuing request.")
requestLogger.Debugf("Request body: %v.", body)

req, err := http.NewRequest(method, c.serverURL+path, bytes.NewBuffer(body))
req, err := http.NewRequestWithContext(ctx, method, c.serverURL+path, bytes.NewBuffer(body))
if err != nil {
return nil, fmt.Errorf("unable to create http request: %w", err)
}
Expand Down
10 changes: 9 additions & 1 deletion tests/e2e/k8s/services/httpecho/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package httpecho

import (
"context"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -48,7 +49,14 @@ func GetEchoValue(cluster *util.KindCluster, server *util.Service) (string, erro
}

url := fmt.Sprintf("http://%s", net.JoinHostPort(cluster.IP(), strconv.Itoa(int(port))))
resp, err := client.Get(url)
ctx := context.Background()
req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, http.NoBody)
if err != nil {
// handle error
return "", fmt.Errorf("cannot create request: %w", err)
}

resp, err := client.Do(req)
if err != nil {
if errors.Is(err, syscall.ECONNREFUSED) {
return "", &services.ConnectionRefusedError{}
Expand Down
16 changes: 8 additions & 8 deletions tests/e2e/k8s/util/k8s_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,29 +37,29 @@ func replaceOnce(s, search, replace string) (string, error) {
return strings.ReplaceAll(s, search, replace), nil
}

func (f *Fabric) generateK8SYAML(p *peer, cfg *PeerConfig) (string, error) {
func (f *Fabric) generateK8SYAML(pr *peer, cfg *PeerConfig) (string, error) {
logLevel := "info"
if os.Getenv("DEBUG") == "1" {
logLevel = "debug"
}

k8sYAMLBytes, err := platform.K8SConfig(&platform.Config{
Peer: p.cluster.Name(),
Peer: pr.cluster.Name(),
FabricCertificate: f.cert,
PeerCertificate: p.peerCert,
CACertificate: p.caCert,
PeerCertificate: pr.peerCert,
CACertificate: pr.caCert,
Controlplanes: cfg.Controlplanes,
ControlplaneCertificate: p.controlplaneCert,
DataplaneCertificate: p.dataplaneCert,
ControlplaneCertificate: pr.controlplaneCert,
DataplaneCertificate: pr.dataplaneCert,
Dataplanes: cfg.Dataplanes,
DataplaneType: cfg.DataplaneType,
LogLevel: logLevel,
ContainerRegistry: "",
Namespace: f.namespace,
Tag: "latest",
PeerLabels: map[string]string{
ClusterNameLabel: p.cluster.name,
PeerIPLabel: p.cluster.ip,
ClusterNameLabel: pr.cluster.name,
PeerIPLabel: pr.cluster.ip,
},
})
if err != nil {
Expand Down
Loading