Skip to content

Commit 796a01d

Browse files
authored
Fix missing plugin name prefix in --set hint for apps init (#5089)
## Summary - The validation error for missing required resources suggested `--set postgres.branch=value`, but the `--set` format requires three parts: `plugin.resourceKey.field`. The correct hint is `--set lakebase.postgres.branch=value`. - Added `PluginName` field to the `Resource` struct (populated during `CollectResources`/`CollectOptionalResources`) and included it in the error message. Thanks to @philip for spotting this bug! ## Test plan - [x] Existing `TestParseSetValues*` tests pass - [x] `libs/apps/manifest` tests pass This pull request and its description were written by Isaac.
1 parent 3eab8c8 commit 796a01d

3 files changed

Lines changed: 83 additions & 15 deletions

File tree

cmd/apps/init.go

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,28 @@ func pluginHasResourceField(p *manifest.Plugin, resourceKey, fieldName string) b
282282
return false
283283
}
284284

285+
// validateRequiredResources checks that all required resources have at least one
286+
// value in resourceValues. Returns an error with a --set hint if any are missing.
287+
func validateRequiredResources(resources []manifest.Resource, resourceValues map[string]string) error {
288+
for _, r := range resources {
289+
found := false
290+
for k := range resourceValues {
291+
if strings.HasPrefix(k, r.Key()+".") {
292+
found = true
293+
break
294+
}
295+
}
296+
if !found {
297+
fieldHint := "id"
298+
if names := r.FieldNames(); len(names) > 0 {
299+
fieldHint = names[0]
300+
}
301+
return fmt.Errorf("missing required resource %q for selected plugins (use --set %s.%s.%s=value)", r.Alias, r.PluginName, r.Key(), fieldHint)
302+
}
303+
}
304+
return nil
305+
}
306+
285307
// tmplBundle holds the generated bundle configuration strings.
286308
type tmplBundle struct {
287309
Variables string
@@ -967,21 +989,8 @@ func runCreate(ctx context.Context, opts createOptions) error {
967989
}
968990

969991
// Validate that all required resources are provided.
970-
for _, r := range resources {
971-
found := false
972-
for k := range resourceValues {
973-
if strings.HasPrefix(k, r.Key()+".") {
974-
found = true
975-
break
976-
}
977-
}
978-
if !found {
979-
fieldHint := "id"
980-
if names := r.FieldNames(); len(names) > 0 {
981-
fieldHint = names[0]
982-
}
983-
return fmt.Errorf("missing required resource %q for selected plugins (use --set %s.%s=value)", r.Alias, r.Key(), fieldHint)
984-
}
992+
if err := validateRequiredResources(resources, resourceValues); err != nil {
993+
return err
985994
}
986995
}
987996

cmd/apps/init_test.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,59 @@ func TestPluginHasResourceField(t *testing.T) {
773773
assert.False(t, pluginHasResourceField(p, "nosuch", "id"))
774774
}
775775

776+
func TestValidateRequiredResources(t *testing.T) {
777+
tests := []struct {
778+
name string
779+
resources []manifest.Resource
780+
resourceValues map[string]string
781+
wantErr string
782+
}{
783+
{
784+
name: "all provided",
785+
resources: []manifest.Resource{
786+
{Alias: "SQL Warehouse", ResourceKey: "sql-warehouse", PluginName: "analytics"},
787+
},
788+
resourceValues: map[string]string{"sql-warehouse.id": "abc"},
789+
},
790+
{
791+
name: "missing resource with fields includes plugin prefix in hint",
792+
resources: []manifest.Resource{
793+
{
794+
Alias: "Postgres",
795+
ResourceKey: "postgres",
796+
PluginName: "lakebase",
797+
Fields: map[string]manifest.ResourceField{
798+
"branch": {Description: "branch"},
799+
"database": {Description: "database"},
800+
},
801+
},
802+
},
803+
resourceValues: map[string]string{},
804+
wantErr: `use --set lakebase.postgres.branch=value`,
805+
},
806+
{
807+
name: "missing resource without fields defaults to id",
808+
resources: []manifest.Resource{
809+
{Alias: "SQL Warehouse", ResourceKey: "sql-warehouse", PluginName: "analytics"},
810+
},
811+
resourceValues: map[string]string{},
812+
wantErr: `use --set analytics.sql-warehouse.id=value`,
813+
},
814+
}
815+
816+
for _, tc := range tests {
817+
t.Run(tc.name, func(t *testing.T) {
818+
err := validateRequiredResources(tc.resources, tc.resourceValues)
819+
if tc.wantErr == "" {
820+
assert.NoError(t, err)
821+
} else {
822+
require.Error(t, err)
823+
assert.Contains(t, err.Error(), tc.wantErr)
824+
}
825+
})
826+
}
827+
}
828+
776829
func TestAppendUnique(t *testing.T) {
777830
result := appendUnique([]string{"a", "b"}, "b", "c", "a", "d")
778831
assert.Equal(t, []string{"a", "b", "c", "d"}, result)

libs/apps/manifest/manifest.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ type Resource struct {
3535
Permission string `json:"permission"` // e.g., "CAN_USE"
3636
Fields map[string]ResourceField `json:"fields"` // field definitions with env var mappings
3737

38+
// PluginName is the machine name of the plugin (e.g., "lakebase").
39+
// Set during resource collection. Not part of the JSON manifest.
40+
PluginName string `json:"-"`
41+
3842
// PluginDisplayName is set during resource collection to identify which
3943
// plugin requires this resource. Not part of the JSON manifest.
4044
PluginDisplayName string `json:"-"`
@@ -218,6 +222,7 @@ func (m *Manifest) CollectResources(pluginNames []string) []Resource {
218222
key := r.Type + ":" + r.Key()
219223
if !seen[key] {
220224
seen[key] = true
225+
r.PluginName = name
221226
r.PluginDisplayName = plugin.DisplayName
222227
resources = append(resources, r)
223228
}
@@ -246,6 +251,7 @@ func (m *Manifest) CollectOptionalResources(pluginNames []string) []Resource {
246251
key := r.Type + ":" + r.Key()
247252
if !seen[key] {
248253
seen[key] = true
254+
r.PluginName = name
249255
r.PluginDisplayName = plugin.DisplayName
250256
resources = append(resources, r)
251257
}

0 commit comments

Comments
 (0)