Skip to content

Commit 8fdd396

Browse files
committed
fixup: Revise for consistency with task-docmenting parent branch
Change-Id: I08aae438ee4e238ba919cad55fddacd8eb68a095
1 parent 0aade30 commit 8fdd396

1 file changed

Lines changed: 127 additions & 71 deletions

File tree

docs/ci.md

Lines changed: 127 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,81 +1,81 @@
11
# CI
22

3-
This document explains the CI rules that apply across the repo.
3+
This document explains how to maintain the CI conventions that apply across the
4+
repo.
45

56
Use it when changing workflows, adding actions, or reviewing CI changes that
67
should follow a shared repo rule rather than a one-off local choice.
78

89
## Table of contents
910

10-
- [Overview](#overview)
11-
- [Usage](#usage)
12-
- [Conceptual model](#conceptual-model)
13-
- [Use stable repo entrypoints](#use-stable-repo-entrypoints)
11+
- [Principles](#principles)
12+
- [How CI should work in this repo](#how-ci-should-work-in-this-repo)
13+
- [Keep repo behavior in stable entrypoints](#keep-repo-behavior-in-stable-entrypoints)
1414
- [Pin runner labels](#pin-runner-labels)
1515
- [Pin third-party actions](#pin-third-party-actions)
16-
- [Only `actions/*` may use major tags](#only-actionsmay-use-major-tags)
17-
- [How these rules are enforced today](#how-these-rules-are-enforced-today)
16+
- [Only `actions/*` may use major tags](#only-actions-may-use-major-tags)
17+
- [Some CI-only setup still belongs in workflows](#some-ci-only-setup-still-belongs-in-workflows)
18+
- [Checks enforce common mistakes](#checks-enforce-common-mistakes)
19+
- [Examples](#examples)
20+
- [Good: a workflow runs a named task](#good-a-workflow-runs-a-named-task)
21+
- [Good: a workflow uses a local action](#good-a-workflow-uses-a-local-action)
22+
- [Avoid: workflow-only repo behavior](#avoid-workflow-only-repo-behavior)
1823
- [Maintainer guidance](#maintainer-guidance)
1924
- [References](#references)
2025

21-
## Overview
26+
## Principles
2227

23-
The main CI goals in this repo are:
24-
25-
- keep workflow behavior easy to find and review
26-
- make CI runs easier to reproduce locally when that is useful
27-
- make upgrades to runners and external actions intentional
28+
- Keeping CI behavior in the repo makes it **easy to find and review**.
29+
- A contributor can inspect how a workflow launches an operation without
30+
reconstructing a shell command from YAML.
31+
- Reusing normal repo entrypoints makes CI **easier to reproduce locally**.
32+
- Local execution may still need CI-only infrastructure or credentials.
33+
- Pinning runner images and external actions makes upgrades **intentional**.
34+
- A change to either is visible in a reviewed repo change rather than
35+
happening when an upstream floating reference moves.
2836

2937
Feature-specific CI details should live with the feature that owns them. This
30-
page is only for rules that apply across workflows and local composite actions.
31-
32-
## Usage
38+
page is for rules that apply across workflows and local composite actions.
3339

34-
Use this document when you are:
40+
## How CI should work in this repo
3541

36-
- adding or reviewing a GitHub Actions workflow
37-
- adding or updating a GitHub Action from outside this repo
38-
- deciding whether workflow logic belongs in a task, a local action, or a
39-
small CI-only helper
40-
- deciding whether a workflow is doing too much work in YAML
42+
These guidelines describe the repo's default approach to CI, not rules that
43+
forbid every exception. When a different design is warranted, it should still
44+
be easy to explain in terms of keeping behavior discoverable, reproducible when
45+
useful, and intentional to upgrade.
4146

42-
For task-specific guidance, including when workflows should call
43-
`./scripts/run_task.sh`, see [Tasks](./tasks.md).
44-
45-
## Conceptual model
46-
47-
### Use stable repo entrypoints
47+
### Keep repo behavior in stable entrypoints
4848

4949
When CI runs behavior defined in this repo, it should usually call an existing
5050
repo command, task, script, or action instead of building the real command in
51-
the workflow.
51+
workflow YAML.
5252

5353
In practice, that usually means one of:
5454

5555
- a named task launched with `./scripts/run_task.sh`
5656
- a local composite action under `./.github/actions/`
57-
- a small CI-only helper script when the behavior is truly CI glue and not a
57+
- a small CI-only helper script when the behavior is CI glue rather than a
5858
developer-facing entrypoint
5959

60-
When a workflow does call a CI-only helper script directly, the script should
61-
follow the `scripts/workflow-*.sh` naming convention. That is the documented
62-
exception to the normal preference for repo entrypoints such as tasks and local
63-
actions, and `scripts/actionlint.sh` checks for it.
60+
When a workflow calls a CI-only helper script directly, the script should use
61+
the `scripts/workflow-*.sh` naming convention. This is the documented exception
62+
to the normal preference for repo entrypoints such as tasks and local actions.
6463

65-
Keep the supported behavior visible in the repo instead of hiding it in
66-
workflow YAML.
64+
Most non-trivial shell logic should live under [`scripts/`](../scripts/), while
65+
logic specific to GitHub Actions or packaging may live under
66+
[`.github/`](../.github/). Keeping it out of workflow YAML makes the real
67+
behavior easier to inspect, check, test, reuse, and review.
6768

68-
This helps with review and maintenance. If CI runs behavior defined in this repo, a
69-
contributor should usually be able to find how it is launched in the tree and rerun it
70-
or inspect it directly.
69+
For task-specific guidance, including when workflows should call
70+
`./scripts/run_task.sh`, see [Tasks](./tasks.md).
7171

7272
### Pin runner labels
7373

7474
GitHub-hosted runner labels should use explicit versions such as `ubuntu-24.04`
7575
or `macos-26`, not floating labels such as `ubuntu-latest` or `macos-latest`.
7676

77-
Floating labels can change underneath the default branch without any matching
78-
repo change. Explicit labels make runner image upgrades a reviewed repo change.
77+
Floating labels can change underneath the default branch without a matching repo
78+
change. Explicit labels make runner image upgrades a reviewed repo change.
7979

8080
### Pin third-party actions
8181

@@ -85,69 +85,125 @@ This repo treats actions in two groups:
8585
- third-party actions outside this repository
8686

8787
Local actions under `./.github/actions/` are part of this repo, so there is no
88-
external version to pin.
89-
90-
For third-party actions, the default rule is to pin a full commit SHA. This
88+
external version to pin. Other actions should use a full commit SHA. This
9189
reduces supply-chain risk and makes upgrades explicit.
9290

9391
### Only `actions/*` may use major tags
9492

95-
This repo currently allows `actions/*` to use floating major version tags
96-
instead of full SHAs.
93+
This repo allows `actions/*` to use floating major version tags instead of full
94+
SHAs. This is a repo convention, not a universal rule.
9795

98-
This is a repo convention, not a universal rule.
96+
Pinning every action by SHA would make every upgrade more explicit, but it would
97+
also add churn for the one provider the repo already has to trust to run GitHub
98+
Actions. The current rule is:
9999

100-
The trade-off is that pinning every action by SHA would make every upgrade more
101-
explicit, but it would also add more churn for the one provider this repo
102-
already has to trust as part of running GitHub Actions at all.
100+
- `actions/*` may use major tags such as `@v5` or `@v6`
101+
- every other third-party action must use a full commit SHA
103102

104-
So the current repo rule is:
103+
The repo may still pin an `actions/*` action by SHA in a specific case. The
104+
important rule is that actions outside `actions/*` do not float.
105105

106-
- `actions/*` may use major tags such as `@v5` or `@v6`
107-
- all other third-party actions should be pinned to full commit SHAs
106+
### Some CI-only setup still belongs in workflows
107+
108+
CI sometimes needs setup that a local command does not. That is fine when it
109+
adapts the execution environment rather than changes the repo operation being
110+
run.
108111

109-
The repo may still choose to pin an `actions/*` action by SHA in a specific
110-
case. The repo rule is that everything outside `actions/*` should not float.
112+
For example, workflows may need runner choice, container setup, artifact
113+
uploads, secrets, or a `workflow_dispatch` input. This is normal CI setup, not
114+
a reason to move the underlying operation into YAML.
111115

112-
### How these rules are enforced today
116+
### Checks enforce common mistakes
113117

114-
Current automated checks include:
118+
[`scripts/actionlint.sh`](../scripts/actionlint.sh) checks several common CI
119+
mistakes:
115120

116121
- workflows should usually invoke stable tasks or local actions instead of
117122
calling most `scripts/*` paths directly
118123
- direct workflow script calls are allowed only for CI-only helpers following
119124
the `scripts/workflow-*.sh` naming convention
120125
- workflow task invocations should not change task behavior by passing extra
121126
option flags after `--`
127+
- third-party actions outside `actions/*` must be pinned by full SHA
122128
- floating GitHub-hosted runner labels are forbidden
123129

124-
Those checks live in [`scripts/actionlint.sh`](../scripts/actionlint.sh).
130+
These checks are practical guardrails, not a complete model of workflow
131+
behavior. Code review still decides whether a new action or exception is
132+
appropriate.
133+
134+
## Examples
135+
136+
### Good: a workflow runs a named task
137+
138+
In [`.github/workflows/ci.yml`](../.github/workflows/ci.yml), the unit-test job
139+
runs:
140+
141+
```bash
142+
./scripts/run_task.sh test-unit
143+
```
144+
145+
A contributor can run the same operation locally with:
146+
147+
```bash
148+
task test-unit
149+
```
150+
151+
The task may call scripts, tools, or other tasks underneath, but the workflow
152+
is not hiding extra behavior.
153+
154+
### Good: a workflow uses a local action
155+
156+
In [`.github/workflows/ci.yml`](../.github/workflows/ci.yml), end-to-end jobs
157+
use the local `run-monitored-tmpnet-cmd` action. The action owns shared
158+
monitoring and artifact behavior, while the workflow selects the named task:
159+
160+
```yaml
161+
- uses: ./.github/actions/run-monitored-tmpnet-cmd
162+
with:
163+
run: ./scripts/run_task.sh test-e2e-ci
164+
```
165+
166+
This keeps shared GitHub Actions glue in one local action instead of duplicating
167+
it across jobs.
168+
169+
### Avoid: workflow-only repo behavior
170+
171+
Avoid defining the only real form of a repo operation through workflow commands,
172+
environment variables, or task configuration. For example:
173+
174+
```yaml
175+
- run: ./scripts/run_task.sh test-e2e
176+
env:
177+
E2E_RUNTIME: kube
178+
```
125179
126-
The action-pinning rule is checked by repo lint: `actions/*` may use major
127-
version tags, while other third-party actions should be pinned by full SHA.
128-
Code review still matters for deciding whether a newly introduced action is
129-
appropriate in the first place.
180+
That forces a contributor to reconstruct workflow state instead of rerunning a
181+
clear named task. In a case like this, prefer a dedicated task such as
182+
`test-e2e-kube-ci`. Workflow environment configuration is still appropriate when
183+
it is needed only to run that task in CI.
130184

131185
## Maintainer guidance
132186

133187
When changing CI, work through these questions:
134188

135189
1. what existing repo command, task, script, or action should this use?
136190
2. should that be a task, a local action, or a CI-only helper?
137-
3. does this change add an external action, and if so how should it be pinned?
138-
4. is the workflow mostly calling existing repo behavior, or is it starting to
139-
define the only real way to invoke a given check?
191+
3. does this change add an external action, and if so is it pinned correctly?
192+
4. does the workflow use an explicit runner image?
193+
5. is the workflow calling existing repo behavior, or defining the only real
194+
way to invoke a check?
140195

141196
Useful review questions:
142197

143198
- can an engineer find the real behavior without reading too much workflow
144199
glue?
145-
- if this workflow adds a third-party action outside `actions/*`, is it pinned
146-
by full SHA?
147-
- does the workflow use an explicit runner image instead of a floating
148-
`*-latest` label?
149-
- if a task is involved, is CI calling a stable named task rather than shaping
150-
the behavior in the workflow?
200+
- can a contributor rerun the same named task locally when that is useful?
201+
- if a task has an argument, is that argument part of its normal local
202+
interface rather than hidden CI-only configuration?
203+
- if the workflow calls a script directly, is it truly CI-only glue with the
204+
`scripts/workflow-*.sh` naming convention?
205+
- if a workflow adds a third-party action outside `actions/*`, is it pinned by
206+
full SHA?
151207

152208
If the same CI rule keeps showing up in review comments, that is usually a sign
153209
that it should be documented here and possibly automated.

0 commit comments

Comments
 (0)