Skip to content

Cleanup client interface #662

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

Merged
inteon merged 2 commits into from
Jun 11, 2025
Merged

Cleanup client interface #662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
469b1d1
cleanup client interface
inteon Jun 7, 2025
45824bc
Apply suggested changes
inteon Jun 10, 2025
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
53 changes: 10 additions & 43 deletions pkg/agent/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package agent

import (
"bytes"
"context"
"encoding/json"
"errors"
Expand Down Expand Up @@ -425,7 +424,8 @@ func postData(ctx context.Context, config CombinedConfig, preflightClient client

log.V(logs.Debug).Info("Posting data", "baseURL", baseURL)

if config.TLSPKMode == VenafiCloudKeypair || config.TLSPKMode == VenafiCloudVenafiConnection {
switch config.TLSPKMode { // nolint:exhaustive
case VenafiCloudKeypair, VenafiCloudVenafiConnection:
// orgID and clusterID are not required for Venafi Cloud auth
err := preflightClient.PostDataReadingsWithOptions(ctx, readings, client.Options{
ClusterName: config.ClusterID,
Expand All @@ -437,55 +437,22 @@ func postData(ctx context.Context, config CombinedConfig, preflightClient client
log.Info("Data sent successfully")

return nil
}

if config.OrganizationID == "" {
Copy link
Contributor Author

@inteon inteon Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This will not be true when TLSPKMode is JetstackSecureOAuth or JetstackSecureAPIToken, since this is checked here:

jetstack-secure/pkg/agent/config.go

Lines 598 to 600 in 5aab733

if cfg.OrganizationID == "" {
errs = multierror.Append(errs, fmt.Errorf("organization_id is required"))
}

Copy link
Contributor Author

@inteon inteon Jun 10, 2025

Choose a reason for hiding this comment

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

And the other two modes (VenafiCloudKeypair and VenafiCloudVenafiConnection are handled above already).

Copy link
Member

@maelvls maelvls Jun 10, 2025

Choose a reason for hiding this comment

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

I imagine that the test coverage would show that this code path is never run? Either way, this doesn't seem to be tested anyways. I'm fine with removing this.

data, err := json.Marshal(readings)
if err != nil {
return fmt.Errorf("Cannot marshal readings: %+v", err)
}

// log and collect metrics about the upload size
metric := metricPayloadSize.With(
prometheus.Labels{"organization": config.OrganizationID, "cluster": config.ClusterID},
)
metric.Set(float64(len(data)))
log.Info("Data readings", "uploadSize", len(data))
path := config.EndpointPath
if path == "" {
path = "/api/v1/datareadings"
}
res, err := preflightClient.Post(ctx, path, bytes.NewBuffer(data))

case JetstackSecureOAuth, JetstackSecureAPIToken:
err := preflightClient.PostDataReadingsWithOptions(ctx, readings, client.Options{
OrgID: config.OrganizationID,
ClusterID: config.ClusterID,
})
if err != nil {
return fmt.Errorf("failed to post data: %+v", err)
}
if code := res.StatusCode; code < 200 || code >= 300 {
errorContent := ""
body, _ := io.ReadAll(res.Body)
if err == nil {
errorContent = string(body)
}
defer res.Body.Close()

return fmt.Errorf("received response with status code %d. Body: [%s]", code, errorContent)
return fmt.Errorf("post to server failed: %+v", err)
}
log.Info("Data sent successfully")

return err
}

if config.ClusterID == "" {
return fmt.Errorf("post to server failed: missing clusterID from agent configuration")
default:
return fmt.Errorf("not implemented for mode %s", config.TLSPKMode)
}

err := preflightClient.PostDataReadings(ctx, config.OrganizationID, config.ClusterID, readings)
if err != nil {
return fmt.Errorf("post to server failed: %+v", err)
}
log.Info("Data sent successfully")

return nil
}

// listenAndServe starts the supplied HTTP server and stops it gracefully when
Expand Down
4 changes: 0 additions & 4 deletions pkg/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ package client
import (
"context"
"fmt"
"io"
"net/http"
"strings"

"github.com/jetstack/preflight/api"
Expand All @@ -30,9 +28,7 @@ type (

// The Client interface describes types that perform requests against the Jetstack Secure backend.
Client interface {
PostDataReadings(ctx context.Context, orgID, clusterID string, readings []*api.DataReading) error
PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, options Options) error
Post(ctx context.Context, path string, body io.Reader) (*http.Response, error)
}

// The Credentials interface describes methods for credential types to implement for verification.
Expand Down
8 changes: 4 additions & 4 deletions pkg/client/client_api_token.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@ func NewAPITokenClient(agentMetadata *api.AgentMetadata, apiToken, baseURL strin
// PostDataReadingsWithOptions uploads the slice of api.DataReading to the Jetstack Secure backend to be processed for later
// viewing in the user-interface.
func (c *APITokenClient) PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, opts Options) error {
return c.PostDataReadings(ctx, opts.OrgID, opts.ClusterID, readings)
return c.postDataReadings(ctx, opts.OrgID, opts.ClusterID, readings)
}

// PostDataReadings uploads the slice of api.DataReading to the Jetstack Secure backend to be processed for later
// viewing in the user-interface.
func (c *APITokenClient) PostDataReadings(ctx context.Context, orgID, clusterID string, readings []*api.DataReading) error {
func (c *APITokenClient) postDataReadings(ctx context.Context, orgID, clusterID string, readings []*api.DataReading) error {
payload := api.DataReadingsPost{
AgentMetadata: c.agentMetadata,
DataGatherTime: time.Now().UTC(),
Expand All @@ -64,7 +64,7 @@ func (c *APITokenClient) PostDataReadings(ctx context.Context, orgID, clusterID
return err
}

res, err := c.Post(ctx, filepath.Join("/api/v1/org", orgID, "datareadings", clusterID), bytes.NewBuffer(data))
res, err := c.post(ctx, filepath.Join("/api/v1/org", orgID, "datareadings", clusterID), bytes.NewBuffer(data))
if err != nil {
return err
}
Expand All @@ -84,7 +84,7 @@ func (c *APITokenClient) PostDataReadings(ctx context.Context, orgID, clusterID
}

// Post performs an HTTP POST request.
func (c *APITokenClient) Post(ctx context.Context, path string, body io.Reader) (*http.Response, error) {
func (c *APITokenClient) post(ctx context.Context, path string, body io.Reader) (*http.Response, error) {
req, err := http.NewRequestWithContext(ctx, http.MethodPost, fullURL(c.baseURL, path), body)
if err != nil {
return nil, err
Expand Down
8 changes: 4 additions & 4 deletions pkg/client/client_oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,12 @@ func NewOAuthClient(agentMetadata *api.AgentMetadata, credentials *OAuthCredenti
}

func (c *OAuthClient) PostDataReadingsWithOptions(ctx context.Context, readings []*api.DataReading, opts Options) error {
return c.PostDataReadings(ctx, opts.OrgID, opts.ClusterID, readings)
return c.postDataReadings(ctx, opts.OrgID, opts.ClusterID, readings)
}

// PostDataReadings uploads the slice of api.DataReading to the Jetstack Secure backend to be processed for later
// viewing in the user-interface.
func (c *OAuthClient) PostDataReadings(ctx context.Context, orgID, clusterID string, readings []*api.DataReading) error {
func (c *OAuthClient) postDataReadings(ctx context.Context, orgID, clusterID string, readings []*api.DataReading) error {
payload := api.DataReadingsPost{
AgentMetadata: c.agentMetadata,
DataGatherTime: time.Now().UTC(),
Expand All @@ -120,7 +120,7 @@ func (c *OAuthClient) PostDataReadings(ctx context.Context, orgID, clusterID str
return err
}

res, err := c.Post(ctx, filepath.Join("/api/v1/org", orgID, "datareadings", clusterID), bytes.NewBuffer(data))
res, err := c.post(ctx, filepath.Join("/api/v1/org", orgID, "datareadings", clusterID), bytes.NewBuffer(data))
if err != nil {
return err
}
Expand All @@ -140,7 +140,7 @@ func (c *OAuthClient) PostDataReadings(ctx context.Context, orgID, clusterID str
}

// Post performs an HTTP POST request.
func (c *OAuthClient) Post(ctx context.Context, path string, body io.Reader) (*http.Response, error) {
func (c *OAuthClient) post(ctx context.Context, path string, body io.Reader) (*http.Response, error) {
token, err := c.getValidAccessToken(ctx)
if err != nil {
return nil, err
Expand Down
40 changes: 2 additions & 38 deletions pkg/client/client_venafi_cloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,43 +205,7 @@ func (c *VenafiCloudClient) PostDataReadingsWithOptions(ctx context.Context, rea
}
venafiCloudUploadURL.RawQuery = query.Encode()

res, err := c.Post(ctx, venafiCloudUploadURL.String(), bytes.NewBuffer(data))
if err != nil {
return err
}
defer res.Body.Close()

if code := res.StatusCode; code < 200 || code >= 300 {
errorContent := ""
body, err := io.ReadAll(res.Body)
if err == nil {
errorContent = string(body)
}
return fmt.Errorf("received response with status code %d. Body: [%s]", code, errorContent)
}

return nil
}

// PostDataReadings uploads the slice of api.DataReading to the Venafi Cloud backend to be processed for later
// viewing in the user-interface.
func (c *VenafiCloudClient) PostDataReadings(ctx context.Context, _ string, _ string, readings []*api.DataReading) error {
// orgID and clusterID are ignored in Venafi Cloud auth

payload := api.DataReadingsPost{
AgentMetadata: c.agentMetadata,
DataGatherTime: time.Now().UTC(),
DataReadings: readings,
}
data, err := json.Marshal(payload)
if err != nil {
return err
}

if !strings.HasSuffix(c.uploadPath, "/") {
c.uploadPath = fmt.Sprintf("%s/", c.uploadPath)
}
res, err := c.Post(ctx, filepath.Join(c.uploadPath, c.uploaderID), bytes.NewBuffer(data))
res, err := c.post(ctx, venafiCloudUploadURL.String(), bytes.NewBuffer(data))
if err != nil {
return err
}
Expand All @@ -260,7 +224,7 @@ func (c *VenafiCloudClient) PostDataReadings(ctx context.Context, _ string, _ st
}

// Post performs an HTTP POST request.
func (c *VenafiCloudClient) Post(ctx context.Context, path string, body io.Reader) (*http.Response, error) {
func (c *VenafiCloudClient) post(ctx context.Context, path string, body io.Reader) (*http.Response, error) {
token, err := c.getValidAccessToken(ctx)
if err != nil {
return nil, err
Expand Down
15 changes: 0 additions & 15 deletions pkg/client/client_venconn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,18 +196,3 @@ func (c *VenConnClient) PostDataReadingsWithOptions(ctx context.Context, reading

return nil
}

// PostDataReadings isn't implemented for Venafi Cloud. This is because Venafi
// Cloud needs a `clusterName` and `clusterDescription`, but this function can
// only pass `orgID` and `clusterID` which are both useless in Venafi Cloud. Use
// PostDataReadingsWithOptions instead.
func (c *VenConnClient) PostDataReadings(_ context.Context, _orgID, _clusterID string, readings []*api.DataReading) error {
return fmt.Errorf("programmer mistake: PostDataReadings is not implemented for Venafi Cloud")
}

// Post isn't implemented for Venafi Cloud because /v1/tlspk/upload/clusterdata
// requires using the query parameters `name` and `description` which can't be
// set using Post. Use PostDataReadingsWithOptions instead.
func (c *VenConnClient) Post(_ context.Context, path string, body io.Reader) (*http.Response, error) {
return nil, fmt.Errorf("programmer mistake: Post is not implemented for Venafi Cloud")
}
23 changes: 4 additions & 19 deletions pkg/internal/cyberark/identity/mock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,25 +197,10 @@ func (mis *mockIdentityServer) handleAdvanceAuthentication(w http.ResponseWriter
return
}

if advanceBody.SessionID != successSessionID {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(advanceAuthenticationFailureResponse))
return
}

if advanceBody.MechanismID != successMechanismID {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(advanceAuthenticationFailureResponse))
return
}

if advanceBody.Action != ActionAnswer {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(advanceAuthenticationFailureResponse))
return
}

if advanceBody.Answer != successPassword {
if advanceBody.SessionID != successSessionID ||
advanceBody.MechanismID != successMechanismID ||
advanceBody.Action != ActionAnswer ||
advanceBody.Answer != successPassword {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte(advanceAuthenticationFailureResponse))
return
Expand Down