Skip to content

Commit 53aa3bc

Browse files
isometryisometry
committed
feat(migrate): add legacy check and status migration
Add migration for legacy "expr"/"expression" keys to "check" in checks. Convert legacy HTTP "status" field to CEL check expressions. Add tests.
1 parent 0894630 commit 53aa3bc

File tree

2 files changed

+347
-0
lines changed

2 files changed

+347
-0
lines changed

‎pkg/commands/migrate/migrate.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"fmt"
66
"os"
7+
"strings"
78

89
"github.com/spf13/cobra"
910
"go.yaml.in/yaml/v3"
@@ -56,6 +57,75 @@ func transformRest(config map[string]any) {
5657
delete(config, "request")
5758
}
5859

60+
// transformChecks rewrites legacy "expr"/"expression" keys to "check" in checks entries.
61+
func transformChecks(config map[string]any) {
62+
checksValue, ok := config["checks"]
63+
if !ok {
64+
return
65+
}
66+
checksSlice, ok := checksValue.([]any)
67+
if !ok {
68+
return
69+
}
70+
for _, item := range checksSlice {
71+
itemMap, ok := item.(map[string]any)
72+
if !ok {
73+
continue
74+
}
75+
for _, oldKey := range []string{"expr", "expression"} {
76+
if val, ok := itemMap[oldKey]; ok {
77+
itemMap["check"] = val
78+
delete(itemMap, oldKey)
79+
break
80+
}
81+
}
82+
}
83+
}
84+
85+
// transformHTTPStatus converts a legacy HTTP "status" field to a CEL check expression.
86+
// Returns a migration note if a transformation was performed.
87+
func transformHTTPStatus(config map[string]any) string {
88+
statusValue, ok := config["status"]
89+
if !ok {
90+
return ""
91+
}
92+
statusSlice, ok := statusValue.([]any)
93+
if !ok {
94+
return ""
95+
}
96+
if len(statusSlice) == 0 {
97+
return ""
98+
}
99+
100+
// Build CEL expression
101+
var expr string
102+
if len(statusSlice) == 1 {
103+
expr = fmt.Sprintf("response.status == %v", statusSlice[0])
104+
} else {
105+
parts := make([]string, len(statusSlice))
106+
for i, s := range statusSlice {
107+
parts[i] = fmt.Sprintf("%v", s)
108+
}
109+
expr = fmt.Sprintf("response.status in [%s]", strings.Join(parts, ", "))
110+
}
111+
112+
// Create check entry
113+
checkEntry := map[string]any{
114+
"check": expr,
115+
"message": "unexpected HTTP status",
116+
}
117+
118+
// Append to existing checks or create new
119+
if existingChecks, ok := config["checks"].([]any); ok {
120+
config["checks"] = append(existingChecks, checkEntry)
121+
} else {
122+
config["checks"] = []any{checkEntry}
123+
}
124+
125+
delete(config, "status")
126+
return fmt.Sprintf("status %v -> CEL check: %s", statusSlice, expr)
127+
}
128+
59129
func run(cmd *cobra.Command, args []string) error {
60130
v := phctx.Viper(cmd.Context())
61131
cliflags.BindFlags(cmd, v)
@@ -140,6 +210,16 @@ func run(cmd *cobra.Command, args []string) error {
140210
transformRest(inst.config)
141211
}
142212

213+
// Convert legacy HTTP status field to CEL check
214+
if providerType == "http" {
215+
if note := transformHTTPStatus(inst.config); note != "" {
216+
renames = append(renames, fmt.Sprintf(" %s: %s", finalName, note))
217+
}
218+
}
219+
220+
// Rewrite legacy check expression keys (expr/expression -> check)
221+
transformChecks(inst.config)
222+
143223
// Build new instance config, routing keys to framework level or spec
144224
newInstance := make(map[string]any)
145225
newInstance["type"] = providerType

‎pkg/commands/migrate/migrate_test.go‎

Lines changed: 267 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
11
package migrate
22

33
import (
4+
"bytes"
5+
"context"
6+
"os"
7+
"path/filepath"
8+
"strings"
49
"testing"
10+
11+
"go.yaml.in/yaml/v3"
12+
13+
"github.com/isometry/platform-health/pkg/phctx"
514
)
615

716
func TestTransformRest(t *testing.T) {
@@ -72,3 +81,261 @@ func TestTypeRewrites(t *testing.T) {
7281
t.Errorf("expected rest -> http rewrite, got %q (ok=%v)", newType, ok)
7382
}
7483
}
84+
85+
func TestTransformChecks(t *testing.T) {
86+
tests := []struct {
87+
name string
88+
input map[string]any
89+
wantCh string // expected "check" value in first entry
90+
}{
91+
{
92+
name: "rewrites expr to check",
93+
input: map[string]any{
94+
"checks": []any{
95+
map[string]any{"expr": "response.status == 200", "message": "bad status"},
96+
},
97+
},
98+
wantCh: "response.status == 200",
99+
},
100+
{
101+
name: "rewrites expression to check",
102+
input: map[string]any{
103+
"checks": []any{
104+
map[string]any{"expression": "response.status == 200", "message": "bad status"},
105+
},
106+
},
107+
wantCh: "response.status == 200",
108+
},
109+
{
110+
name: "leaves existing check key alone",
111+
input: map[string]any{
112+
"checks": []any{
113+
map[string]any{"check": "response.status == 200", "message": "ok"},
114+
},
115+
},
116+
wantCh: "response.status == 200",
117+
},
118+
{
119+
name: "no checks key is a no-op",
120+
input: map[string]any{"url": "https://example.com"},
121+
},
122+
{
123+
name: "non-slice checks is a no-op",
124+
input: map[string]any{"checks": "not-a-slice"},
125+
},
126+
}
127+
128+
for _, tt := range tests {
129+
t.Run(tt.name, func(t *testing.T) {
130+
transformChecks(tt.input)
131+
if tt.wantCh == "" {
132+
return
133+
}
134+
checksSlice, ok := tt.input["checks"].([]any)
135+
if !ok {
136+
t.Fatal("checks is not []any after transform")
137+
}
138+
entry, ok := checksSlice[0].(map[string]any)
139+
if !ok {
140+
t.Fatal("first check entry is not map[string]any")
141+
}
142+
if got, ok := entry["check"].(string); !ok || got != tt.wantCh {
143+
t.Errorf("expected check=%q, got %q", tt.wantCh, got)
144+
}
145+
if _, ok := entry["expr"]; ok {
146+
t.Error("expected expr key to be removed")
147+
}
148+
if _, ok := entry["expression"]; ok {
149+
t.Error("expected expression key to be removed")
150+
}
151+
})
152+
}
153+
}
154+
155+
func TestTransformHTTPStatus(t *testing.T) {
156+
tests := []struct {
157+
name string
158+
input map[string]any
159+
wantExpr string
160+
wantNote bool
161+
wantCount int // expected number of checks entries
162+
}{
163+
{
164+
name: "single status value",
165+
input: map[string]any{"status": []any{200}},
166+
wantExpr: "response.status == 200",
167+
wantNote: true,
168+
wantCount: 1,
169+
},
170+
{
171+
name: "multiple status values",
172+
input: map[string]any{"status": []any{200, 201}},
173+
wantExpr: "response.status in [200, 201]",
174+
wantNote: true,
175+
wantCount: 1,
176+
},
177+
{
178+
name: "merges with existing checks",
179+
input: map[string]any{
180+
"status": []any{200},
181+
"checks": []any{
182+
map[string]any{"check": "response.json.ok == true", "message": "not ok"},
183+
},
184+
},
185+
wantExpr: "response.status == 200",
186+
wantNote: true,
187+
wantCount: 2,
188+
},
189+
{
190+
name: "no status key is a no-op",
191+
input: map[string]any{"url": "https://example.com"},
192+
},
193+
{
194+
name: "non-slice status is a no-op",
195+
input: map[string]any{"status": "200"},
196+
},
197+
{
198+
name: "empty status slice is a no-op",
199+
input: map[string]any{"status": []any{}},
200+
},
201+
}
202+
203+
for _, tt := range tests {
204+
t.Run(tt.name, func(t *testing.T) {
205+
note := transformHTTPStatus(tt.input)
206+
if tt.wantNote && note == "" {
207+
t.Error("expected a migration note, got empty")
208+
}
209+
if !tt.wantNote && note != "" {
210+
t.Errorf("expected no migration note, got %q", note)
211+
}
212+
if tt.wantExpr == "" {
213+
return
214+
}
215+
// status key should be removed
216+
if _, ok := tt.input["status"]; ok {
217+
t.Error("expected status key to be removed")
218+
}
219+
checksSlice, ok := tt.input["checks"].([]any)
220+
if !ok {
221+
t.Fatal("checks is not []any after transform")
222+
}
223+
if len(checksSlice) != tt.wantCount {
224+
t.Fatalf("expected %d checks entries, got %d", tt.wantCount, len(checksSlice))
225+
}
226+
// The generated check is always the last entry
227+
last, ok := checksSlice[len(checksSlice)-1].(map[string]any)
228+
if !ok {
229+
t.Fatal("last check entry is not map[string]any")
230+
}
231+
if got := last["check"]; got != tt.wantExpr {
232+
t.Errorf("expected check=%q, got %q", tt.wantExpr, got)
233+
}
234+
})
235+
}
236+
}
237+
238+
func TestRunIntegration(t *testing.T) {
239+
const input = `http:
240+
- name: google
241+
url: https://google.com
242+
status: [200]
243+
rest:
244+
- name: api
245+
request:
246+
url: https://api.example.com/health
247+
checks:
248+
- expr: "response.status == 200"
249+
message: "HTTP request failed"
250+
- expr: 'response.json.status == "ok"'
251+
message: "API unhealthy"
252+
`
253+
254+
// Write input to temp file
255+
tmpDir := t.TempDir()
256+
inputPath := filepath.Join(tmpDir, "old-config.yaml")
257+
if err := os.WriteFile(inputPath, []byte(input), 0644); err != nil {
258+
t.Fatal(err)
259+
}
260+
261+
// Build command with context
262+
cmd := New()
263+
ctx := phctx.ContextWithViper(context.Background(), phctx.NewViper())
264+
cmd.SetContext(ctx)
265+
266+
var stdout, stderr bytes.Buffer
267+
cmd.SetOut(&stdout)
268+
cmd.SetErr(&stderr)
269+
cmd.SetArgs([]string{inputPath})
270+
271+
if err := cmd.Execute(); err != nil {
272+
t.Fatalf("run failed: %v\nstderr: %s", err, stderr.String())
273+
}
274+
275+
// Parse output
276+
var result map[string]any
277+
if err := yaml.Unmarshal(stdout.Bytes(), &result); err != nil {
278+
t.Fatalf("failed to parse output YAML: %v\noutput: %s", err, stdout.String())
279+
}
280+
281+
components, ok := result["components"].(map[string]any)
282+
if !ok {
283+
t.Fatalf("expected components map, got %T", result["components"])
284+
}
285+
286+
// Verify google component: status should be converted to check, no spec.status
287+
google, ok := components["google"].(map[string]any)
288+
if !ok {
289+
t.Fatal("missing google component")
290+
}
291+
if google["type"] != "http" {
292+
t.Errorf("expected google type=http, got %v", google["type"])
293+
}
294+
if spec, ok := google["spec"].(map[string]any); ok {
295+
if _, hasStatus := spec["status"]; hasStatus {
296+
t.Error("expected status to be removed from spec")
297+
}
298+
}
299+
googleChecks, ok := google["checks"].([]any)
300+
if !ok || len(googleChecks) == 0 {
301+
t.Fatal("expected google to have checks")
302+
}
303+
firstCheck, ok := googleChecks[0].(map[string]any)
304+
if !ok {
305+
t.Fatal("first google check is not a map")
306+
}
307+
if firstCheck["check"] != "response.status == 200" {
308+
t.Errorf("expected status CEL check, got %v", firstCheck["check"])
309+
}
310+
311+
// Verify api component: expr should be rewritten to check
312+
api, ok := components["api"].(map[string]any)
313+
if !ok {
314+
t.Fatal("missing api component")
315+
}
316+
if api["type"] != "http" {
317+
t.Errorf("expected api type=http, got %v", api["type"])
318+
}
319+
apiChecks, ok := api["checks"].([]any)
320+
if !ok || len(apiChecks) != 2 {
321+
t.Fatalf("expected api to have 2 checks, got %v", apiChecks)
322+
}
323+
for i, c := range apiChecks {
324+
entry, ok := c.(map[string]any)
325+
if !ok {
326+
t.Fatalf("api check[%d] is not a map", i)
327+
}
328+
if _, hasExpr := entry["expr"]; hasExpr {
329+
t.Errorf("api check[%d] still has legacy 'expr' key", i)
330+
}
331+
if _, hasCheck := entry["check"]; !hasCheck {
332+
t.Errorf("api check[%d] missing 'check' key", i)
333+
}
334+
}
335+
336+
// Verify stderr contains migration notes
337+
stderrStr := stderr.String()
338+
if !strings.Contains(stderrStr, "migration notes") {
339+
t.Errorf("expected migration notes in stderr, got: %s", stderrStr)
340+
}
341+
}

0 commit comments

Comments
 (0)