[quality] comment-only prow.yaml throws YAMLException instead of loading as an empty configuration (parseProwConfig trim guard misses zero-document input)

Cerrado Apto para principiantes
#393 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Ya se ha fusionado un pull request relacionado.

  • #415 de @mrbobbytables — fusionado

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
65/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
github-actions, nodejs, typescript
Área
tooling

Línea de trabajo

Empieza en parseProwConfig en src/utils/config.ts (alrededor de las líneas 369-374), donde la guarda trim() solo detecta entradas en blanco antes de yaml.load. Reproduce el problema con el fragmento de js-yaml del issue y luego amplía la rama de documento vacío para cubrir entradas que solo contienen comentarios. Está listo cuando # prow configuration\n devuelve {} y los casos añadidos pasan en tests/utils/config.test.ts y tests/bundle/emptyConfigDocument.test.ts, con npm run build && npm run pack ejecutado para que dist/ quede actualizado.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

agent/quality hive/hosted-available-lke648397-260827-5q9t needs-kind quality testing

Finding

A comment-only prow.yaml breaks every configuration-reading command. parseProwConfig (src/utils/config.ts:369-374) treats an empty document as an all-empty configuration:

const loaded: unknown = text.trim() === '' ? undefined : yaml.load(text)
if (loaded === undefined || loaded === null) {
  return {}
}

but the guard only catches a blank/whitespace file. js-yaml 5 (^5.4.2, bumped in #126) throws YAMLException: expected a document, but the input is empty from load() whenever the input contains zero documents, and a file holding only comments is such an input:

$ node -e "const y=require('js-yaml');for (const t of ['','# c\n','---\n','  \n']){try{console.log(JSON.stringify(t),'->',y.load(t))}catch(e){console.log(JSON.stringify(t),'THROWS',e.message)}}"
""      THROWS expected a document, but the input is empty
"# c\n" THROWS expected a document, but the input is empty
"---\n" -> null
"  \n"  THROWS expected a document, but the input is empty

So '' and ' \n' are rescued by the trim() guard, '---\n' loads as null, and '# c\n' falls through to yaml.load and throws. Driven through dist/index.js on main @ aa6a0f8 with .github/prow.yaml = # prow configuration\n# nothing enabled yet\n:

  • /lifecycle stale → exit 1, ::error::TypeError: error handling issue comment: Error: could not get labels from yaml: YAMLException: expected a document, but the input is empty — instead of the built-in frozen,stale,rotten defaults that a blank file yields.
  • /kind cleanup → the same YAMLException, instead of kind: yaml malformed, expected 'kind' top level key.

The same path runs for the org tier (<owner>/.project / <owner>/.github:prow.yaml), so a placeholder, comment-only org file breaks every repository in the organization.

History: #126's commit message accepted the new throw on empty documents ("the failure mode is unchanged"); #144 then deliberately made empty documents an all-empty configuration (return {}) and unit-tested '', '---\n' and ' \n' (__tests__/utils/config.test.ts:420-426) but not a comment-only document, which is the one shape users actually commit as a placeholder.

Recommendation

One deliverable: make the empty-document arm cover zero-document input regardless of comments. Either

  • in parseProwConfig, replace the trim() guard with a zero-document check, e.g. const docs = yaml.loadAll(text); const loaded = docs.length === 0 ? undefined : docs[0] (and keep the existing expected a mapping at the top level rejection for multi-document or non-mapping input), or catch the YAMLException whose message starts with expected a document, but the input is empty and return {}
  • add ['a comment-only file', '# prow configuration\n'] to the treats %s as an all-empty config cases in __tests__/utils/config.test.ts
  • add the same case to the emptyDocuments table of __tests__/bundle/emptyConfigDocument.test.ts (added by the companion PR; it drives the blank and --- arms through dist/index.js and names this issue for the case it leaves out)
  • npm run build && npm run pack so the dist/ drift gate passes

This touches production source (src/utils/config.ts), so it is not a PR this lane opens; it is filed for a maintainer.

Priority

  • Impact: medium (a comment-only placeholder config silently disables every label command for a repository or, at the org tier, every repository)
  • Effort: low

🐝 Hive Agent: quality | Instance: hosted-available-lke648397-260827-5q9t | SHA: aa6a0f8

— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88

Lenguaje dominante
TypeScript
Estrellas
132
Forks
24
Merge medio
1 d 6 h
PR fusionados (30 d)
240

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de cncf/prow-github-actions

Todos los issues de cncf/prow-github-actions

Issues similares

Más issues de TypeScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.