Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 31 additions & 3 deletions pkg/devfile/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,35 @@ func DevfileSamplesHandler(w http.ResponseWriter, r *http.Request) {
w.Write(sampleIndex)
}

// parseDevfileWithFallback attempts to parse the devfile with full parent/plugin
// resolution (flattened). If that fails (e.g. the parent registry is unreachable or
// the OCI pull times out), it retries without flattening. The unflattened parse
// still provides all locally-defined components and commands, which is sufficient
// for the outer-loop resource generation the console backend performs.
func parseDevfileWithFallback(devfileContentBytes []byte, httpTimeout *int) (parser.DevfileObj, error) {
devfileObj, _, err := devfile.ParseDevfileAndValidate(parser.ParserArgs{
Data: devfileContentBytes,
HTTPTimeout: httpTimeout,
})
if err == nil {
return devfileObj, nil
}

klog.Warningf("Flattened devfile parse failed, retrying without parent resolution: %v", err)

flattenedDevfile := false
devfileObj, _, err = devfile.ParseDevfileAndValidate(parser.ParserArgs{
Data: devfileContentBytes,
HTTPTimeout: httpTimeout,
FlattenedDevfile: &flattenedDevfile,
})
if err != nil {
return parser.DevfileObj{}, err
}

return devfileObj, nil
}

func DevfileHandler(w http.ResponseWriter, r *http.Request) {
var (
data DevfileForm
Expand All @@ -48,11 +77,10 @@ func DevfileHandler(w http.ResponseWriter, r *http.Request) {
return
}

// Get devfile content and parse it using a library call in the future
devfileContentBytes := []byte(data.Devfile.DevfileContent)
//reduce the http request and response timeouts on the devfile library parser to 10s
httpTimeout := 10
devfileObj, _, err = devfile.ParseDevfileAndValidate(parser.ParserArgs{Data: devfileContentBytes, HTTPTimeout: &httpTimeout})

devfileObj, err = parseDevfileWithFallback(devfileContentBytes, &httpTimeout)
if err != nil {
errMsg := "Failed to parse devfile:"
if strings.Contains(err.Error(), "schemaVersion not present in devfile") {
Expand Down
137 changes: 137 additions & 0 deletions pkg/devfile/handler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package devfile

import (
"testing"

"github.com/stretchr/testify/assert"
)

const validDevfileNoParent = `
schemaVersion: 2.2.0
metadata:
name: test-app
attributes:
alpha.dockerimage-port: 8080
components:
- name: image-build
image:
imageName: test-image:latest
dockerfile:
uri: Dockerfile
buildContext: .
- name: kubernetes-deploy
kubernetes:
inlined: |
kind: Deployment
apiVersion: apps/v1
metadata:
name: my-deploy
spec:
replicas: 1
selector:
matchLabels:
app: test-app
template:
metadata:
labels:
app: test-app
spec:
containers:
- name: my-container
image: test-image:latest
commands:
- id: build-image
apply:
component: image-build
- id: deployk8s
apply:
component: kubernetes-deploy
- id: deploy
composite:
commands:
- build-image
- deployk8s
group:
kind: deploy
isDefault: true
`

const devfileWithBadRegistry = `
schemaVersion: 2.2.0
metadata:
name: test
parent:
id: nonexistent-stack
registryUrl: 'https://does-not-exist.invalid'
components:
Comment on lines +65 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Remove external DNS/network dependency from fallback test.

The fallback case currently depends on resolving an external invalid domain (Line 65) and can wait up to 10s (Line 111), which may cause CI flakiness/latency. Use a deterministic local failure target (for example http://127.0.0.1:1) and a shorter timeout.

Proposed test hardening
-  registryUrl: 'https://does-not-exist.invalid'
+  registryUrl: 'http://127.0.0.1:1'
@@
-	httpTimeout := 10
+	httpTimeout := 1

Also applies to: 111-125

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/devfile/handler_test.go` around lines 65 - 66, The test currently relies
on an external DNS name via the registryUrl YAML value and a long wait (about
10s), causing CI flakiness; update the fallback test to use a deterministic
local failure target (replace registryUrl: 'https://does-not-exist.invalid' with
'http://127.0.0.1:1') and shorten the related wait/timeout (the ~10s backoff
used in the test) to a small value like 200–500ms so the test fails fast; locate
the registryUrl string in the test fixture and the timeout/wait variable used
around the fallback assertion and make these two changes.

- name: image-build
image:
imageName: test:latest
dockerfile:
uri: Dockerfile
buildContext: .
- name: kubernetes-deploy
kubernetes:
inlined: |
kind: Deployment
apiVersion: apps/v1
metadata:
name: test-deploy
spec:
replicas: 1
selector:
matchLabels:
app: test
template:
metadata:
labels:
app: test
spec:
containers:
- name: test
image: test:latest
commands:
- id: build-image
apply:
component: image-build
- id: deployk8s
apply:
component: kubernetes-deploy
- id: deploy
composite:
commands:
- build-image
- deployk8s
group:
kind: deploy
isDefault: true
`

func TestParseDevfileWithFallback(t *testing.T) {
httpTimeout := 10

t.Run("devfile without parent parses on first attempt", func(t *testing.T) {
devfileObj, err := parseDevfileWithFallback([]byte(validDevfileNoParent), &httpTimeout)
assert.NoError(t, err)

components, err := GetDeployComponents(devfileObj)
assert.NoError(t, err)
assert.Contains(t, components, "image-build")
assert.Contains(t, components, "kubernetes-deploy")
})

t.Run("devfile with unreachable parent falls back to unflattened parse", func(t *testing.T) {
devfileObj, err := parseDevfileWithFallback([]byte(devfileWithBadRegistry), &httpTimeout)
assert.NoError(t, err, "fallback to unflattened parse should succeed")

components, err := GetDeployComponents(devfileObj)
assert.NoError(t, err)
assert.Contains(t, components, "image-build")
assert.Contains(t, components, "kubernetes-deploy")
})

t.Run("completely invalid devfile fails both attempts", func(t *testing.T) {
_, err := parseDevfileWithFallback([]byte("not valid yaml: ["), &httpTimeout)
assert.Error(t, err)
})
}