Skip to content

Commit 525bf0e

Browse files
authored
feat: always upload bad specs (#1730)
1 parent 575de89 commit 525bf0e

File tree

4 files changed

+65
-12
lines changed

4 files changed

+65
-12
lines changed

internal/run/source.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,31 +177,42 @@ func (w *Workflow) RunSource(ctx context.Context, parentStep *workflowTracking.W
177177
}
178178
sourceRes.OutputPath = outputLocation
179179

180+
var lintingErr error
180181
if !w.SkipLinting {
181182
w.OnSourceResult(sourceRes, SourceStepLint)
182183
sourceRes.LintResult, err = w.validateDocument(ctx, rootStep, sourceID, currentDocument, rulesetToUse, w.ProjectDir, targetLanguage)
183184
if err != nil {
184-
return "", sourceRes, &LintingError{Err: err, Document: currentDocument}
185+
lintingErr = &LintingError{Err: err, Document: currentDocument}
185186
}
186187
}
187188

189+
var diagnoseErr error
188190
step := rootStep.NewSubstep("Diagnosing OpenAPI")
189-
sourceRes.Diagnosis, err = suggest.Diagnose(ctx, currentDocument)
190-
if err != nil {
191+
sourceRes.Diagnosis, diagnoseErr = suggest.Diagnose(ctx, currentDocument)
192+
if diagnoseErr != nil {
191193
step.Fail()
192-
return "", sourceRes, err
194+
} else {
195+
step.Succeed()
193196
}
194-
step.Succeed()
195197

196198
w.OnSourceResult(sourceRes, SourceStepUpload)
197199

198-
if !w.SkipSnapshot {
199-
err = w.snapshotSource(ctx, rootStep, sourceID, source, sourceRes)
200+
// Upload if validation passed OR if the spec looks like an OpenAPI spec (even if invalid)
201+
if !w.SkipSnapshot && (lintingErr == nil || validation.LooksLikeAnOpenAPISpec(currentDocument)) {
202+
err = w.snapshotSource(ctx, rootStep, sourceID, source, sourceRes, lintingErr)
200203
if err != nil && !errors.Is(err, ocicommon.ErrAccessGated) {
201204
logger.Warnf("failed to snapshot source: %s", err.Error())
202205
}
203206
}
204207

208+
// Return errors after snapshot attempt - prefer linting error over diagnose error
209+
if lintingErr != nil {
210+
return "", sourceRes, lintingErr
211+
}
212+
if diagnoseErr != nil {
213+
return "", sourceRes, diagnoseErr
214+
}
215+
205216
// If the source has a previous tracked revision, compute changes against it
206217
if w.lockfileOld != nil && !w.SkipChangeReport {
207218
if targetLockOld, ok := w.lockfileOld.Targets[targetID]; ok && !utils.IsZeroTelemetryOrganization(ctx) {

internal/run/sourceTracking.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func (w *Workflow) computeChanges(ctx context.Context, rootStep *workflowTrackin
157157
return computedChanges, err
158158
}
159159

160-
func (w *Workflow) snapshotSource(ctx context.Context, parentStep *workflowTracking.WorkflowStep, sourceID string, source workflow.Source, sourceResult *SourceResult) (err error) {
160+
func (w *Workflow) snapshotSource(ctx context.Context, parentStep *workflowTracking.WorkflowStep, sourceID string, source workflow.Source, sourceResult *SourceResult, lintingErr error) (err error) {
161161
registryStep := parentStep.NewSubstep("Tracking OpenAPI Changes")
162162

163163
if !registry.IsRegistryEnabled(ctx) {
@@ -204,7 +204,7 @@ func (w *Workflow) snapshotSource(ctx context.Context, parentStep *workflowTrack
204204
apiKey = key
205205
}
206206

207-
tags, err := w.getRegistryTags(ctx, sourceID)
207+
tags, err := w.getRegistryTags(ctx, sourceID, lintingErr)
208208
if err != nil {
209209
return err
210210
}
@@ -393,8 +393,11 @@ func getAndValidateAPIKey(ctx context.Context, orgSlug, workspaceSlug, registryL
393393
return
394394
}
395395

396-
func (w *Workflow) getRegistryTags(ctx context.Context, sourceID string) ([]string, error) {
397-
tags := []string{"latest"}
396+
func (w *Workflow) getRegistryTags(ctx context.Context, sourceID string, lintingErr error) ([]string, error) {
397+
var tags []string = []string{"latest"}
398+
if lintingErr != nil {
399+
return tags, nil
400+
}
398401
if env.IsGithubAction() {
399402
// implicitly add branch tag
400403
var branch string

internal/run/target.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ func (w *Workflow) snapshotCodeSamples(ctx context.Context, parentStep *workflow
316316
return
317317
}
318318

319-
tags, err := w.getRegistryTags(ctx, "")
319+
tags, err := w.getRegistryTags(ctx, "", nil)
320320
if err != nil {
321321
return
322322
}

internal/validation/openapi.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"io"
7+
"os"
78
"slices"
89
"strings"
910

@@ -23,6 +24,7 @@ import (
2324
"github.com/speakeasy-api/speakeasy/internal/interactivity"
2425
"github.com/speakeasy-api/speakeasy/internal/log"
2526
"go.uber.org/zap"
27+
"gopkg.in/yaml.v3"
2628
)
2729

2830
// OutputLimits defines the limits for validation output.
@@ -384,3 +386,40 @@ func generateReport(ctx context.Context, res validationResult) (reports.ReportRe
384386
reportBytes := res.GenerateReport()
385387
return reports.UploadReport(ctx, reportBytes, shared.TypeLinting)
386388
}
389+
390+
// LooksLikeAnOpenAPISpec performs a basic check to determine if a file appears to be an OpenAPI spec.
391+
// It checks:
392+
// 1. The file can be unmarshaled as YAML (which also covers JSON)
393+
// 2. The file has the top-level keys: "openapi", "info", and "paths"
394+
//
395+
// This is a loose check intended to identify files that are likely OpenAPI specs even if they
396+
// have validation errors.
397+
func LooksLikeAnOpenAPISpec(schemaPath string) bool {
398+
content, err := os.ReadFile(schemaPath)
399+
if err != nil {
400+
return false
401+
}
402+
403+
return LooksLikeAnOpenAPISpecContent(content)
404+
}
405+
406+
// LooksLikeAnOpenAPISpecContent performs a basic check on content to determine if it appears to be an OpenAPI spec.
407+
// It checks:
408+
// 1. The content can be unmarshaled as YAML (which also covers JSON)
409+
// 2. The content has the top-level keys: "openapi", "info", and "paths"
410+
func LooksLikeAnOpenAPISpecContent(content []byte) bool {
411+
var doc map[string]any
412+
if err := yaml.Unmarshal(content, &doc); err != nil {
413+
return false
414+
}
415+
416+
// Check for required top-level keys
417+
requiredKeys := []string{"openapi", "info", "paths"}
418+
for _, key := range requiredKeys {
419+
if _, ok := doc[key]; !ok {
420+
return false
421+
}
422+
}
423+
424+
return true
425+
}

0 commit comments

Comments
 (0)