Skip to content

NLB-7285: Add dataplane_api_key as an optional #1012

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.

Sign up for GitHub

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

Open
Naveen-Gopu-F5 wants to merge 1 commit into
base: main
Choose a base branch
from
+474 −5
Open

NLB-7285: Add dataplane_api_key as an optional #1012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions build/config.yaml.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
# cloud_provider: AWS
# region: us-west-2
# api_endpoint: http://127.0.0.1:8080/api
# optional dataplane_api_key
# dataplane_api_key: your-api-key
Comment on lines +6 to +7
Copy link

@ryepup ryepup Dec 22, 2025

Choose a reason for hiding this comment

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

alternate design idea:

What if instead of dataplane_api_key we kept it very generic, like additional_headers?

So for NGINXaaS integration the user would add:

additional_headers:
  Authorization: ApiKey $BASE_64_KEY

That'd keep the open source codebase very decoupled from our cloud offering, but put a little more work onto the user to get the headers right.

What do y'all think?

# sync_interval: 5s
# upstreams:
# - name: backend-one
Expand All @@ -24,6 +26,8 @@
# subscription_id: my_subscription_id
# resource_group_name: my_resource_group
# api_endpoint: http://127.0.0.1:8080/api
# optional dataplane_api_key
# dataplane_api_key: your-api-key
# sync_interval: 5s
# upstreams:
# - name: backend-one
Expand Down
7 changes: 4 additions & 3 deletions cmd/sync/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import (

// commonConfig stores the configuration parameters common to all providers.
type commonConfig struct {
APIEndpoint string `yaml:"api_endpoint"`
CloudProvider string `yaml:"cloud_provider"`
SyncInterval time.Duration `yaml:"sync_interval"`
APIEndpoint string `yaml:"api_endpoint"`
CloudProvider string `yaml:"cloud_provider"`
DataplaneAPIKey string `yaml:"dataplane_api_key,omitempty"`
SyncInterval time.Duration `yaml:"sync_interval"`
}

func parseCommonConfig(data []byte) (*commonConfig, error) {
Expand Down
62 changes: 60 additions & 2 deletions cmd/sync/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"context"
"encoding/base64"
"flag"
"fmt"
"io"
Expand All @@ -21,7 +22,10 @@ var (
version string
)

const connTimeoutInSecs = 10
const (
connTimeoutInSecs = 10
maxHeaders = 100
)

func main() {
flag.Parse()
Expand Down Expand Up @@ -63,7 +67,7 @@ func main() {
os.Exit(10)
}

httpClient := &http.Client{Timeout: connTimeoutInSecs * time.Second}
httpClient := NewHTTPClient(commonConfig)
nginxClient, err := nginx.NewNginxClient(commonConfig.APIEndpoint, nginx.WithHTTPClient(httpClient))
if err != nil {
log.Printf("Couldn't create NGINX client: %v", err)
Expand Down Expand Up @@ -189,3 +193,57 @@ func getStreamUpstreamServerAddresses(server []nginx.StreamUpstreamServer) []str
}
return streamUpstreamServerAddr
}

// headerTransport wraps an http.RoundTripper and adds custom headers to all requests.
type headerTransport struct {
headers http.Header
transport http.RoundTripper
}

func (t *headerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
clonedReq := req.Clone(req.Context())

for key, values := range t.headers {
for _, value := range values {
clonedReq.Header.Add(key, value)
}
}

if len(clonedReq.Header) > maxHeaders {
return nil, fmt.Errorf("number of headers in request exceeds the maximum allowed (%d)", maxHeaders)
}
Comment on lines +203 to +214
Copy link

@ryepup ryepup Dec 22, 2025

Choose a reason for hiding this comment

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

it's nice to have these sanity checks at the top of function to conserve CPU. If we have too many headers, we've wasted some time cloning and adding.

I think we can tweak the condition and prevent some doomed allocations.

Something like (untested):

Suggested change
func (t *headerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
clonedReq := req.Clone(req.Context())
for key, values := range t.headers {
for _, value := range values {
clonedReq.Header.Add(key, value)
}
}
if len(clonedReq.Header) > maxHeaders {
return nil, fmt.Errorf("number of headers in request exceeds the maximum allowed (%d)", maxHeaders)
}
func (t *headerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
if len(req.Header) + len(t.headers) > maxHeaders {
return nil, fmt.Errorf("number of headers in request exceeds the maximum allowed (%d)", maxHeaders)
}
clonedReq := req.Clone(req.Context())
for key, values := range t.headers {
for _, value := range values {
clonedReq.Header.Add(key, value)
}
}


resp, err := t.transport.RoundTrip(clonedReq)
if err != nil {
return nil, fmt.Errorf("headerTransport RoundTrip failed: %w", err)
}

return resp, nil
}

func NewHTTPClient(cfg *commonConfig) *http.Client {
headers := NewHeaders(cfg)
baseTransport := &http.Transport{}

return &http.Client{
Transport: &headerTransport{
headers: headers,
transport: baseTransport,
},
Timeout: connTimeoutInSecs * time.Second,
}
}
Comment on lines +224 to +235
Copy link

@ryepup ryepup Dec 22, 2025

Choose a reason for hiding this comment

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

we don't want to add a performance penalty if a customer is not using dataplane_api_key.

As written, all requests are going to jump through headerTransport, whether it needs to or not. This creates a little more pressure on the garbage collector and uses more compute than needed. Let's add a check in here to only add headers if we need to.

Maybe something like (untested):

Suggested change
func NewHTTPClient(cfg *commonConfig) *http.Client {
headers := NewHeaders(cfg)
baseTransport := &http.Transport{}
return &http.Client{
Transport: &headerTransport{
headers: headers,
transport: baseTransport,
},
Timeout: connTimeoutInSecs * time.Second,
}
}
func NewHTTPClient(cfg *commonConfig) *http.Client {
var rt http.RoundTripper = &http.Transport{}
if h := NewHeaders(cfg); len(h) > 0 {
rt = &headerTransport{headers: h, transport: rt}
}
return &http.Client{
Transport: rt
Timeout: connTimeoutInSecs * time.Second,
}
}


func NewHeaders(cfg *commonConfig) http.Header {
headers := http.Header{}
headers.Set("Content-Type", "application/json")

if cfg.DataplaneAPIKey != "" {
authValue := "ApiKey " + base64.StdEncoding.EncodeToString([]byte(cfg.DataplaneAPIKey))
headers.Set("Authorization", authValue)
} else {
log.Printf("[optional] DataplaneAPIKey not configured")
}
Comment on lines +244 to +246
Copy link

@ryepup ryepup Dec 22, 2025

Choose a reason for hiding this comment

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

this log message doesn't seem that useful

Suggested change
} else {
log.Printf("[optional] DataplaneAPIKey not configured")
}
}


return headers
}
Loading
Loading