diff --git a/management/server/http/handlers/peers/peers_handler.go b/management/server/http/handlers/peers/peers_handler.go index baea7cba0..d3be176e6 100644 --- a/management/server/http/handlers/peers/peers_handler.go +++ b/management/server/http/handlers/peers/peers_handler.go @@ -58,7 +58,7 @@ func (h *Handler) CreateJob(w http.ResponseWriter, r *http.Request) { req := &api.JobRequest{} 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 } @@ -102,7 +102,6 @@ func (h *Handler) ListJobs(w http.ResponseWriter, r *http.Request) { for _, job := range jobs { resp, err := toSingleJobResponse(job) if err != nil { - log.WithContext(ctx).Errorf("failed xxx: %v", err) util.WriteError(ctx, err, w) return } diff --git a/management/server/peer.go b/management/server/peer.go index dfe79ec6f..18ee68398 100644 --- a/management/server/peer.go +++ b/management/server/peer.go @@ -380,15 +380,15 @@ func (am *DefaultAccountManager) CreatePeerJob(ctx context.Context, accountID, p return err } 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{ - "job_id": job.ID, - "for_peer_id": job.PeerID, - "job_type": job.Type, - "job_status": job.Status, - "job_parameters": job.Parameters, + "job_id": job.ID, + "for_peer_id": job.PeerID, + "job_type": job.Workload.Type, + "job_status": job.Status, + "job_parameters": job.Workload.Parameters, } eventsToStore = func() { @@ -399,9 +399,7 @@ func (am *DefaultAccountManager) CreatePeerJob(ctx context.Context, accountID, p if err != nil { return err } - eventsToStore() - return nil } diff --git a/management/server/store/sql_store.go b/management/server/store/sql_store.go index 59c7d7b0b..de3a01e72 100644 --- a/management/server/store/sql_store.go +++ b/management/server/store/sql_store.go @@ -157,7 +157,7 @@ func (s *SqlStore) GetPeerJobByID(ctx context.Context, accountID, jobID string) Where(accountAndIDQueryCondition, accountID, jobID). First(&job).Error 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 } diff --git a/management/server/types/job.go b/management/server/types/job.go index 85fe9cb7c..4d2be13ee 100644 --- a/management/server/types/job.go +++ b/management/server/types/job.go @@ -7,6 +7,7 @@ import ( "github.com/google/uuid" "github.com/netbirdio/netbird/shared/management/http/api" + "github.com/netbirdio/netbird/shared/management/status" ) type JobStatus string @@ -42,45 +43,47 @@ type Job struct { 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 JobStatus `gorm:"index;type:varchar(50)"` // FailedReason describes why the job failed (if failed) FailedReason string - // Result can contain job output (JSON, URL, etc.) - Result json.RawMessage `gorm:"type:json"` + Workload Workload `gorm:"embedded;embeddedPrefix:workload_"` +} - // Parameters is a JSON blob storing job configuration (untyped) - Parameters json.RawMessage `gorm:"type:json"` +type Workload struct { + 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 func NewJob(triggeredBy, accountID, peerID string, req *api.JobRequest) (*Job, error) { - if req == nil { - return nil, fmt.Errorf("job request cannot be nil") + if req == nil { + return nil, status.Errorf(status.BadRequest, "job request cannot be nil") } // Determine job type jobTypeStr, err := req.Workload.Discriminator() 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) - var params []byte + if jobType == "" { + return nil, status.Errorf(status.BadRequest, "job type is required") + } + + var workload Workload switch jobType { case JobTypeBundle: - params, err = validateDebugBundleJobParams(req.Workload) - if err != nil { - return nil, err + if err := validateBundleParams(req.Workload, &workload); err != nil { + return nil, status.Errorf(status.BadRequest, "%v", err) } default: - return nil, fmt.Errorf("unsupported job type: %s", jobType) + return nil, status.Errorf(status.BadRequest, "unsupported job type: %s", jobType) } return &Job{ @@ -88,79 +91,79 @@ func NewJob(triggeredBy, accountID, peerID string, req *api.JobRequest) (*Job, e TriggeredBy: triggeredBy, PeerID: peerID, AccountID: accountID, - Type: jobType, Status: JobStatusPending, - Parameters: params, CreatedAt: time.Now().UTC(), - Result: []byte("{}"), + Workload: workload, }, nil } func (j *Job) BuildWorkloadResponse(wl *api.WorkloadResponse) error { - switch j.Type { + switch j.Workload.Type { case JobTypeBundle: - return j.buildBundleWorkload(wl) - case JobTypeOther: - return j.buildOtherWorkload(wl) + if err := j.buildBundleWorkload(wl); err != nil { + return status.Errorf(status.InvalidArgument, "%v", err) + } + return nil 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 { - var p api.BundleParameters - - if err := json.Unmarshal(j.Parameters, &p); err != nil { - return fmt.Errorf("invalid parameters for bundle job: %w", err) + var params api.BundleParameters + switch v := j.Workload.Parameters.(type) { + case api.BundleParameters: + 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, ¶ms); 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 - if err := json.Unmarshal(j.Result, &r); err != nil { - return fmt.Errorf("invalid result for bundle job: %w", err) + var result api.BundleResult + switch v := j.Workload.Result.(type) { + 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{ Type: api.WorkloadTypeBundle, - Parameters: p, - Result: r, + Parameters: params, + Result: result, }) } -func (j *Job) buildOtherWorkload(wl *api.WorkloadResponse) 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) { +func validateBundleParams(req api.WorkloadRequest, workload *Workload) error { bundle, err := req.AsBundleWorkloadRequest() 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 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 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) } - - params, err := json.Marshal(bundle.Parameters) - if err != nil { - return nil, fmt.Errorf("failed to marshal workload parameters: %w", err) - } - return params, nil + workload.Parameters = bundle.Parameters + workload.Type = JobTypeBundle + return nil } diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml index dc0dc69e4..b7f4a22d8 100644 --- a/shared/management/http/api/openapi.yml +++ b/shared/management/http/api/openapi.yml @@ -38,22 +38,23 @@ components: type: string enum: - bundle - - other BundleParameters: type: object properties: bundle_for: type: boolean + example: true bundle_for_time: type: integer - format: int64 minimum: 0 + example: 2 log_file_count: type: integer - format: int32 minimum: 0 + example: 100 anonymize: type: boolean + example: false required: - bundle_for - bundle_for_time @@ -64,8 +65,8 @@ components: properties: upload_key: type: string - required: - - upload_key + example: "upload_key_123" + nullable: true BundleWorkloadRequest: type: object properties: @@ -89,61 +90,20 @@ components: - type - parameters - 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: oneOf: - $ref: '#/components/schemas/BundleWorkloadRequest' - - $ref: '#/components/schemas/OtherWorkloadRequest' discriminator: propertyName: type mapping: bundle: '#/components/schemas/BundleWorkloadRequest' - other: '#/components/schemas/OtherWorkloadRequest' WorkloadResponse: oneOf: - $ref: '#/components/schemas/BundleWorkloadResponse' - - $ref: '#/components/schemas/OtherWorkloadResponse' discriminator: propertyName: type mapping: bundle: '#/components/schemas/BundleWorkloadResponse' - other: '#/components/schemas/OtherWorkloadResponse' JobRequest: type: object properties: diff --git a/shared/management/http/api/types.gen.go b/shared/management/http/api/types.gen.go index 4cedc7e57..5c428d48c 100644 --- a/shared/management/http/api/types.gen.go +++ b/shared/management/http/api/types.gen.go @@ -192,7 +192,6 @@ const ( // Defines values for WorkloadType. const ( WorkloadTypeBundle WorkloadType = "bundle" - WorkloadTypeOther WorkloadType = "other" ) // Defines values for GetApiEventsNetworkTrafficParamsType. @@ -356,15 +355,15 @@ type AvailablePorts struct { // BundleParameters defines model for BundleParameters. type BundleParameters struct { - Anonymize bool `json:"anonymize"` - BundleFor bool `json:"bundle_for"` - BundleForTime int64 `json:"bundle_for_time"` - LogFileCount int32 `json:"log_file_count"` + Anonymize bool `json:"anonymize"` + BundleFor bool `json:"bundle_for"` + BundleForTime int `json:"bundle_for_time"` + LogFileCount int `json:"log_file_count"` } // BundleResult defines model for BundleResult. type BundleResult struct { - UploadKey string `json:"upload_key"` + UploadKey *string `json:"upload_key"` } // BundleWorkloadRequest defines model for BundleWorkloadRequest. @@ -1075,29 +1074,6 @@ type OSVersionCheck struct { 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. type Peer struct { // ApprovalRequired (Cloud only) Indicates whether peer needs approval @@ -2099,34 +2075,6 @@ func (t *WorkloadRequest) MergeBundleWorkloadRequest(v BundleWorkloadRequest) er 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) { var discriminator struct { Discriminator string `json:"type"` @@ -2143,8 +2091,6 @@ func (t WorkloadRequest) ValueByDiscriminator() (interface{}, error) { switch discriminator { case "bundle": return t.AsBundleWorkloadRequest() - case "other": - return t.AsOtherWorkloadRequest() default: return nil, errors.New("unknown discriminator value: " + discriminator) } @@ -2188,34 +2134,6 @@ func (t *WorkloadResponse) MergeBundleWorkloadResponse(v BundleWorkloadResponse) 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) { var discriminator struct { Discriminator string `json:"type"` @@ -2232,8 +2150,6 @@ func (t WorkloadResponse) ValueByDiscriminator() (interface{}, error) { switch discriminator { case "bundle": return t.AsBundleWorkloadResponse() - case "other": - return t.AsOtherWorkloadResponse() default: return nil, errors.New("unknown discriminator value: " + discriminator) }