apply feedback 2

This commit is contained in:
aliamerj
2025-08-28 00:19:53 +03:00
parent 9043938233
commit 6c0a46dfd5
6 changed files with 83 additions and 207 deletions
@@ -58,7 +58,7 @@ func (h *Handler) CreateJob(w http.ResponseWriter, r *http.Request) {
req := &api.JobRequest{} req := &api.JobRequest{}
if err := json.NewDecoder(r.Body).Decode(req); err != nil { if err := json.NewDecoder(r.Body).Decode(req); err != nil {
util.WriteError(ctx, err, w) util.WriteErrorResponse("couldn't parse JSON request", http.StatusBadRequest, w)
return return
} }
@@ -102,7 +102,6 @@ func (h *Handler) ListJobs(w http.ResponseWriter, r *http.Request) {
for _, job := range jobs { for _, job := range jobs {
resp, err := toSingleJobResponse(job) resp, err := toSingleJobResponse(job)
if err != nil { if err != nil {
log.WithContext(ctx).Errorf("failed xxx: %v", err)
util.WriteError(ctx, err, w) util.WriteError(ctx, err, w)
return return
} }
+7 -9
View File
@@ -380,15 +380,15 @@ func (am *DefaultAccountManager) CreatePeerJob(ctx context.Context, accountID, p
return err return err
} }
if err := transaction.CreatePeerJob(ctx, job); err != nil { if err := transaction.CreatePeerJob(ctx, job); err != nil {
return fmt.Errorf("failed to save job for peer %s: %w", peer.ID, err) return status.Errorf(status.Internal, "failed to save job for peer %s: %v", peer.ID, err)
} }
jobMeta := map[string]any{ jobMeta := map[string]any{
"job_id": job.ID, "job_id": job.ID,
"for_peer_id": job.PeerID, "for_peer_id": job.PeerID,
"job_type": job.Type, "job_type": job.Workload.Type,
"job_status": job.Status, "job_status": job.Status,
"job_parameters": job.Parameters, "job_parameters": job.Workload.Parameters,
} }
eventsToStore = func() { eventsToStore = func() {
@@ -399,9 +399,7 @@ func (am *DefaultAccountManager) CreatePeerJob(ctx context.Context, accountID, p
if err != nil { if err != nil {
return err return err
} }
eventsToStore() eventsToStore()
return nil return nil
} }
+1 -1
View File
@@ -157,7 +157,7 @@ func (s *SqlStore) GetPeerJobByID(ctx context.Context, accountID, jobID string)
Where(accountAndIDQueryCondition, accountID, jobID). Where(accountAndIDQueryCondition, accountID, jobID).
First(&job).Error First(&job).Error
if errors.Is(err, gorm.ErrRecordNotFound) { if errors.Is(err, gorm.ErrRecordNotFound) {
return nil, fmt.Errorf("job %s not found", jobID) return nil, status.Errorf(status.NotFound, "job %s not found", jobID)
} }
return &job, err return &job, err
} }
+63 -60
View File
@@ -7,6 +7,7 @@ import (
"github.com/google/uuid" "github.com/google/uuid"
"github.com/netbirdio/netbird/shared/management/http/api" "github.com/netbirdio/netbird/shared/management/http/api"
"github.com/netbirdio/netbird/shared/management/status"
) )
type JobStatus string type JobStatus string
@@ -42,45 +43,47 @@ type Job struct {
AccountID string `gorm:"index"` AccountID string `gorm:"index"`
// Type of the job, e.g. "bundle"
Type JobType `gorm:"index;type:varchar(50)"`
// Status of the job: pending, succeeded, failed // Status of the job: pending, succeeded, failed
Status JobStatus `gorm:"index;type:varchar(50)"` Status JobStatus `gorm:"index;type:varchar(50)"`
// FailedReason describes why the job failed (if failed) // FailedReason describes why the job failed (if failed)
FailedReason string FailedReason string
// Result can contain job output (JSON, URL, etc.) Workload Workload `gorm:"embedded;embeddedPrefix:workload_"`
Result json.RawMessage `gorm:"type:json"` }
// Parameters is a JSON blob storing job configuration (untyped) type Workload struct {
Parameters json.RawMessage `gorm:"type:json"` Type JobType `gorm:"index;type:varchar(50)"`
Parameters any `gorm:"serializer:json"`
Result any `gorm:"serializer:json"`
} }
// NewJob creates a new job with default fields and validation // NewJob creates a new job with default fields and validation
func NewJob(triggeredBy, accountID, peerID string, req *api.JobRequest) (*Job, error) { func NewJob(triggeredBy, accountID, peerID string, req *api.JobRequest) (*Job, error) {
if req == nil { if req == nil {
return nil, fmt.Errorf("job request cannot be nil") return nil, status.Errorf(status.BadRequest, "job request cannot be nil")
} }
// Determine job type // Determine job type
jobTypeStr, err := req.Workload.Discriminator() jobTypeStr, err := req.Workload.Discriminator()
if err != nil { if err != nil {
return nil, fmt.Errorf("could not determine job type: %w", err) return nil, status.Errorf(status.BadRequest, "could not determine job type: %v", err)
} }
jobType := JobType(jobTypeStr) jobType := JobType(jobTypeStr)
var params []byte if jobType == "" {
return nil, status.Errorf(status.BadRequest, "job type is required")
}
var workload Workload
switch jobType { switch jobType {
case JobTypeBundle: case JobTypeBundle:
params, err = validateDebugBundleJobParams(req.Workload) if err := validateBundleParams(req.Workload, &workload); err != nil {
if err != nil { return nil, status.Errorf(status.BadRequest, "%v", err)
return nil, err
} }
default: default:
return nil, fmt.Errorf("unsupported job type: %s", jobType) return nil, status.Errorf(status.BadRequest, "unsupported job type: %s", jobType)
} }
return &Job{ return &Job{
@@ -88,79 +91,79 @@ func NewJob(triggeredBy, accountID, peerID string, req *api.JobRequest) (*Job, e
TriggeredBy: triggeredBy, TriggeredBy: triggeredBy,
PeerID: peerID, PeerID: peerID,
AccountID: accountID, AccountID: accountID,
Type: jobType,
Status: JobStatusPending, Status: JobStatusPending,
Parameters: params,
CreatedAt: time.Now().UTC(), CreatedAt: time.Now().UTC(),
Result: []byte("{}"), Workload: workload,
}, nil }, nil
} }
func (j *Job) BuildWorkloadResponse(wl *api.WorkloadResponse) error { func (j *Job) BuildWorkloadResponse(wl *api.WorkloadResponse) error {
switch j.Type { switch j.Workload.Type {
case JobTypeBundle: case JobTypeBundle:
return j.buildBundleWorkload(wl) if err := j.buildBundleWorkload(wl); err != nil {
case JobTypeOther: return status.Errorf(status.InvalidArgument, "%v", err)
return j.buildOtherWorkload(wl) }
return nil
default: default:
return fmt.Errorf("unknown job type: %v", j.Type) return status.Errorf(status.InvalidArgument, "unknown job type: %v", j.Workload.Type)
} }
} }
func (j *Job) buildBundleWorkload(wl *api.WorkloadResponse) error { func (j *Job) buildBundleWorkload(wl *api.WorkloadResponse) error {
var p api.BundleParameters var params api.BundleParameters
switch v := j.Workload.Parameters.(type) {
if err := json.Unmarshal(j.Parameters, &p); err != nil { case api.BundleParameters:
return fmt.Errorf("invalid parameters for bundle job: %w", err) params = v
case map[string]any:
data, err := json.Marshal(v)
if err != nil {
return fmt.Errorf("failed to marshal parameters: %w", err)
}
if err := json.Unmarshal(data, &params); err != nil {
return fmt.Errorf("failed to unmarshal parameters: %w", err)
}
default:
return fmt.Errorf("invalid Bundle Parameters type: %T", j.Workload.Parameters)
} }
var r api.BundleResult var result api.BundleResult
if err := json.Unmarshal(j.Result, &r); err != nil { switch v := j.Workload.Result.(type) {
return fmt.Errorf("invalid result for bundle job: %w", err) case api.BundleResult:
result = v
case map[string]any:
data, err := json.Marshal(v)
if err != nil {
return fmt.Errorf("failed to marshal result: %w", err)
}
if err := json.Unmarshal(data, &result); err != nil {
return fmt.Errorf("failed to unmarshal result: %w", err)
}
case nil:
default:
return fmt.Errorf("invalid Bundle Result type: %T", j.Workload.Result)
} }
return wl.FromBundleWorkloadResponse(api.BundleWorkloadResponse{ return wl.FromBundleWorkloadResponse(api.BundleWorkloadResponse{
Type: api.WorkloadTypeBundle, Type: api.WorkloadTypeBundle,
Parameters: p, Parameters: params,
Result: r, Result: result,
}) })
} }
func (j *Job) buildOtherWorkload(wl *api.WorkloadResponse) error { func validateBundleParams(req api.WorkloadRequest, workload *Workload) error {
var p api.OtherParameters
if err := json.Unmarshal(j.Parameters, &p); err != nil {
return fmt.Errorf("invalid parameters for bundle job: %w", err)
}
var r api.OtherResult
if err := json.Unmarshal(j.Result, &r); err != nil {
return fmt.Errorf("invalid result for bundle job: %w", err)
}
return wl.FromOtherWorkloadResponse(api.OtherWorkloadResponse{
Type: api.WorkloadTypeOther,
Parameters: p,
Result: r,
})
}
func validateDebugBundleJobParams(req api.WorkloadRequest) ([]byte, error) {
bundle, err := req.AsBundleWorkloadRequest() bundle, err := req.AsBundleWorkloadRequest()
if err != nil { if err != nil {
return nil, fmt.Errorf("invalid parameters for bundle job: %w", err) return fmt.Errorf("invalid parameters for bundle job")
} }
// validate bundle_for_time <= 5 minutes // validate bundle_for_time <= 5 minutes
if bundle.Parameters.BundleForTime < 0 || bundle.Parameters.BundleForTime > 5 { if bundle.Parameters.BundleForTime < 0 || bundle.Parameters.BundleForTime > 5 {
return nil, fmt.Errorf("bundle_for_time must be between 0 and 5, got %d", bundle.Parameters.BundleForTime) return fmt.Errorf("bundle_for_time must be between 0 and 5, got %d", bundle.Parameters.BundleForTime)
} }
// validate log-file-count ≥ 1 and ≤ 1000 // validate log-file-count ≥ 1 and ≤ 1000
if bundle.Parameters.LogFileCount < 1 || bundle.Parameters.LogFileCount > 1000 { if bundle.Parameters.LogFileCount < 1 || bundle.Parameters.LogFileCount > 1000 {
return nil, fmt.Errorf("log-file-count must be between 1 and 1000, got %d", bundle.Parameters.LogFileCount) return fmt.Errorf("log-file-count must be between 1 and 1000, got %d", bundle.Parameters.LogFileCount)
} }
workload.Parameters = bundle.Parameters
params, err := json.Marshal(bundle.Parameters) workload.Type = JobTypeBundle
if err != nil { return nil
return nil, fmt.Errorf("failed to marshal workload parameters: %w", err)
}
return params, nil
} }
+6 -46
View File
@@ -38,22 +38,23 @@ components:
type: string type: string
enum: enum:
- bundle - bundle
- other
BundleParameters: BundleParameters:
type: object type: object
properties: properties:
bundle_for: bundle_for:
type: boolean type: boolean
example: true
bundle_for_time: bundle_for_time:
type: integer type: integer
format: int64
minimum: 0 minimum: 0
example: 2
log_file_count: log_file_count:
type: integer type: integer
format: int32
minimum: 0 minimum: 0
example: 100
anonymize: anonymize:
type: boolean type: boolean
example: false
required: required:
- bundle_for - bundle_for
- bundle_for_time - bundle_for_time
@@ -64,8 +65,8 @@ components:
properties: properties:
upload_key: upload_key:
type: string type: string
required: example: "upload_key_123"
- upload_key nullable: true
BundleWorkloadRequest: BundleWorkloadRequest:
type: object type: object
properties: properties:
@@ -89,61 +90,20 @@ components:
- type - type
- parameters - parameters
- result - result
OtherParameters:
type: object
properties:
example_param:
type: string
required:
- example_param
OtherResult:
type: object
properties:
upload_key:
type: string
required:
- upload_key
OtherWorkloadRequest:
type: object
properties:
type:
$ref: '#/components/schemas/WorkloadType'
parameters:
$ref: '#/components/schemas/OtherParameters'
required:
- type
- parameters
OtherWorkloadResponse:
type: object
properties:
type:
$ref: '#/components/schemas/WorkloadType'
parameters:
$ref: '#/components/schemas/OtherParameters'
result:
$ref: '#/components/schemas/OtherResult'
required:
- type
- parameters
- result
WorkloadRequest: WorkloadRequest:
oneOf: oneOf:
- $ref: '#/components/schemas/BundleWorkloadRequest' - $ref: '#/components/schemas/BundleWorkloadRequest'
- $ref: '#/components/schemas/OtherWorkloadRequest'
discriminator: discriminator:
propertyName: type propertyName: type
mapping: mapping:
bundle: '#/components/schemas/BundleWorkloadRequest' bundle: '#/components/schemas/BundleWorkloadRequest'
other: '#/components/schemas/OtherWorkloadRequest'
WorkloadResponse: WorkloadResponse:
oneOf: oneOf:
- $ref: '#/components/schemas/BundleWorkloadResponse' - $ref: '#/components/schemas/BundleWorkloadResponse'
- $ref: '#/components/schemas/OtherWorkloadResponse'
discriminator: discriminator:
propertyName: type propertyName: type
mapping: mapping:
bundle: '#/components/schemas/BundleWorkloadResponse' bundle: '#/components/schemas/BundleWorkloadResponse'
other: '#/components/schemas/OtherWorkloadResponse'
JobRequest: JobRequest:
type: object type: object
properties: properties:
+5 -89
View File
@@ -192,7 +192,6 @@ const (
// Defines values for WorkloadType. // Defines values for WorkloadType.
const ( const (
WorkloadTypeBundle WorkloadType = "bundle" WorkloadTypeBundle WorkloadType = "bundle"
WorkloadTypeOther WorkloadType = "other"
) )
// Defines values for GetApiEventsNetworkTrafficParamsType. // Defines values for GetApiEventsNetworkTrafficParamsType.
@@ -356,15 +355,15 @@ type AvailablePorts struct {
// BundleParameters defines model for BundleParameters. // BundleParameters defines model for BundleParameters.
type BundleParameters struct { type BundleParameters struct {
Anonymize bool `json:"anonymize"` Anonymize bool `json:"anonymize"`
BundleFor bool `json:"bundle_for"` BundleFor bool `json:"bundle_for"`
BundleForTime int64 `json:"bundle_for_time"` BundleForTime int `json:"bundle_for_time"`
LogFileCount int32 `json:"log_file_count"` LogFileCount int `json:"log_file_count"`
} }
// BundleResult defines model for BundleResult. // BundleResult defines model for BundleResult.
type BundleResult struct { type BundleResult struct {
UploadKey string `json:"upload_key"` UploadKey *string `json:"upload_key"`
} }
// BundleWorkloadRequest defines model for BundleWorkloadRequest. // BundleWorkloadRequest defines model for BundleWorkloadRequest.
@@ -1075,29 +1074,6 @@ type OSVersionCheck struct {
Windows *MinKernelVersionCheck `json:"windows,omitempty"` Windows *MinKernelVersionCheck `json:"windows,omitempty"`
} }
// OtherParameters defines model for OtherParameters.
type OtherParameters struct {
ExampleParam string `json:"example_param"`
}
// OtherResult defines model for OtherResult.
type OtherResult struct {
UploadKey string `json:"upload_key"`
}
// OtherWorkloadRequest defines model for OtherWorkloadRequest.
type OtherWorkloadRequest struct {
Parameters OtherParameters `json:"parameters"`
Type WorkloadType `json:"type"`
}
// OtherWorkloadResponse defines model for OtherWorkloadResponse.
type OtherWorkloadResponse struct {
Parameters OtherParameters `json:"parameters"`
Result OtherResult `json:"result"`
Type WorkloadType `json:"type"`
}
// Peer defines model for Peer. // Peer defines model for Peer.
type Peer struct { type Peer struct {
// ApprovalRequired (Cloud only) Indicates whether peer needs approval // ApprovalRequired (Cloud only) Indicates whether peer needs approval
@@ -2099,34 +2075,6 @@ func (t *WorkloadRequest) MergeBundleWorkloadRequest(v BundleWorkloadRequest) er
return err return err
} }
// AsOtherWorkloadRequest returns the union data inside the WorkloadRequest as a OtherWorkloadRequest
func (t WorkloadRequest) AsOtherWorkloadRequest() (OtherWorkloadRequest, error) {
var body OtherWorkloadRequest
err := json.Unmarshal(t.union, &body)
return body, err
}
// FromOtherWorkloadRequest overwrites any union data inside the WorkloadRequest as the provided OtherWorkloadRequest
func (t *WorkloadRequest) FromOtherWorkloadRequest(v OtherWorkloadRequest) error {
v.Type = "other"
b, err := json.Marshal(v)
t.union = b
return err
}
// MergeOtherWorkloadRequest performs a merge with any union data inside the WorkloadRequest, using the provided OtherWorkloadRequest
func (t *WorkloadRequest) MergeOtherWorkloadRequest(v OtherWorkloadRequest) error {
v.Type = "other"
b, err := json.Marshal(v)
if err != nil {
return err
}
merged, err := runtime.JsonMerge(b, t.union)
t.union = merged
return err
}
func (t WorkloadRequest) Discriminator() (string, error) { func (t WorkloadRequest) Discriminator() (string, error) {
var discriminator struct { var discriminator struct {
Discriminator string `json:"type"` Discriminator string `json:"type"`
@@ -2143,8 +2091,6 @@ func (t WorkloadRequest) ValueByDiscriminator() (interface{}, error) {
switch discriminator { switch discriminator {
case "bundle": case "bundle":
return t.AsBundleWorkloadRequest() return t.AsBundleWorkloadRequest()
case "other":
return t.AsOtherWorkloadRequest()
default: default:
return nil, errors.New("unknown discriminator value: " + discriminator) return nil, errors.New("unknown discriminator value: " + discriminator)
} }
@@ -2188,34 +2134,6 @@ func (t *WorkloadResponse) MergeBundleWorkloadResponse(v BundleWorkloadResponse)
return err return err
} }
// AsOtherWorkloadResponse returns the union data inside the WorkloadResponse as a OtherWorkloadResponse
func (t WorkloadResponse) AsOtherWorkloadResponse() (OtherWorkloadResponse, error) {
var body OtherWorkloadResponse
err := json.Unmarshal(t.union, &body)
return body, err
}
// FromOtherWorkloadResponse overwrites any union data inside the WorkloadResponse as the provided OtherWorkloadResponse
func (t *WorkloadResponse) FromOtherWorkloadResponse(v OtherWorkloadResponse) error {
v.Type = "other"
b, err := json.Marshal(v)
t.union = b
return err
}
// MergeOtherWorkloadResponse performs a merge with any union data inside the WorkloadResponse, using the provided OtherWorkloadResponse
func (t *WorkloadResponse) MergeOtherWorkloadResponse(v OtherWorkloadResponse) error {
v.Type = "other"
b, err := json.Marshal(v)
if err != nil {
return err
}
merged, err := runtime.JsonMerge(b, t.union)
t.union = merged
return err
}
func (t WorkloadResponse) Discriminator() (string, error) { func (t WorkloadResponse) Discriminator() (string, error) {
var discriminator struct { var discriminator struct {
Discriminator string `json:"type"` Discriminator string `json:"type"`
@@ -2232,8 +2150,6 @@ func (t WorkloadResponse) ValueByDiscriminator() (interface{}, error) {
switch discriminator { switch discriminator {
case "bundle": case "bundle":
return t.AsBundleWorkloadResponse() return t.AsBundleWorkloadResponse()
case "other":
return t.AsOtherWorkloadResponse()
default: default:
return nil, errors.New("unknown discriminator value: " + discriminator) return nil, errors.New("unknown discriminator value: " + discriminator)
} }