Skip to content
Merged
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
9 changes: 8 additions & 1 deletion docs/schemas/task-catalog-v1.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,20 @@
"dynamic_property": {
"type": "object",
"additionalProperties": false,
"required": ["prefix", "probeable"],
"required": ["prefix", "probeable", "scopes"],
"properties": {
"prefix": { "type": "string", "description": "What a member's name starts with." },
"probeable": {
"type": "boolean",
"description": "Whether the plugin's report surfaces a row per set member, so docket can read the value back. `false` means a recipe using the family reports drift on every run and never converges."
},
"scopes": {
"type": "array",
"items": { "enum": ["app", "global"] },
"minItems": 1,
"uniqueItems": true,
"description": "Where a member of the family may be used. Using one in a scope it does not list is rejected at validate time, matching dokku's own rejection."
},
"sensitive": {
"const": true,
"description": "Members' values are treated as secrets."
Expand Down
8 changes: 7 additions & 1 deletion docs/task-catalog.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,8 @@ set subcommand rather than through its report schema. Those are published as pre
consumer does not reject a legal recipe:

```json
"dynamic": [ { "prefix": "dns-provider-", "probeable": true, "sensitive": true } ]
"dynamic": [ { "prefix": "dns-provider-", "probeable": true, "sensitive": true,
"scopes": ["global"] } ]
```

`probeable: false` means docket cannot read those values back, so a recipe using that family
Expand All @@ -203,6 +204,11 @@ reports drift on every run and never converges.
them into an input on export. It is independent of `probeable` - a credential docket cannot read
back is still masked on the way out.

`scopes` reads exactly as it does on an enumerable property: a family listed only under `global` may
be used with `global: true` and is rejected for an app, matching dokku's own rejection. The
`dns-provider-*` credentials are global-only on traefik and app-or-global on letsencrypt, so a
consumer that ignored this would accept a recipe dokku refuses.

A name can also be absent from `properties` because another task manages it. That is a different
answer from "no such property", and the catalog says so rather than leaving a consumer to report an
unknown name and offer a list it will never be in:
Expand Down
2 changes: 1 addition & 1 deletion docs/tasks/dokku_letsencrypt_property.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Keyed by `app`, `global`, and `property`. Fields left empty are omitted from the
| `lego-docker-options` | app, global | `lego-docker-options` | `global-lego-docker-options` |
| `server` | app, global | `server` | `global-server` |

Names starting with `dns-provider-` are also accepted. dokku validates them through `letsencrypt:set` rather than through its report schema, so they cannot be listed above. The plugin reports each one it has been given, so they probe for drift like any listed property. Their values are treated as secrets and masked.
Names starting with `dns-provider-` are also accepted in the app and global scopes. dokku validates them through `letsencrypt:set` rather than through its report schema, so they cannot be listed above. The plugin reports each one it has been given, so they probe for drift like any listed property. Their values are treated as secrets and masked.

## Examples

Expand Down
14 changes: 12 additions & 2 deletions docs/tasks/dokku_traefik_property.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Supported.

## Probe support

Partial - the mapped properties are probed; the dynamic `dns-provider-*` family has no report key and plans as drift on every run.
Supported.

## Identity

Expand Down Expand Up @@ -48,7 +48,7 @@ Keyed by `app`, `global`, and `property`. Fields left empty are omitted from the
| `letsencrypt-server` | global | | `global-letsencrypt-server` |
| `log-level` | global | | `global-log-level` |

Names starting with `dns-provider-` are also accepted. dokku validates them through `traefik:set` rather than through its report schema, so they cannot be listed above. The plugin does not report them, so they are applied on every run and never converge. Their values are treated as secrets and masked.
Names starting with `dns-provider-` are also accepted in the global scope only. dokku validates them through `traefik:set` rather than through its report schema, so they cannot be listed above. The plugin reports each one it has been given, so they probe for drift like any listed property. Their values are treated as secrets and masked.

## Examples

Expand All @@ -72,6 +72,16 @@ dokku_traefik_property:
value: INFO
```

### Setting a dns-provider-* env var globally

```yaml
dokku_traefik_property:
app: ""
global: true
property: dns-provider-CLOUDFLARE_DNS_API_TOKEN
value: cf-token
```

### Clearing the letsencrypt email globally

```yaml
Expand Down
18 changes: 13 additions & 5 deletions docs/writing-tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -362,12 +362,20 @@ Those are declared in `dynamicPropertyFamilies` in `tasks/properties.go`, which
validation accept a name the table has never heard of, and is published to consumers so a linter
does not reject a legal recipe. How they plan depends on the plugin:

- The plugin reports the family (letsencrypt on 0.25.0+ emits a row per set property): declare it
`Probeable`, and the scope keys are synthesized so the property probes like any mapped one.
Because the row only exists once the property has a value, an absent row reads as unset. Note the
minimum plugin version in `Requirements()`.
- The plugin reports the family (letsencrypt on 0.25.0+ and traefik on dokku 0.38.27+ emit a row per
set property): declare it `Probeable`, and the scope keys are synthesized so the property probes
like any mapped one. Because the row only exists once the property has a value, an absent row reads
as unset. Note the minimum plugin version in `Requirements()`.
- The plugin does not report it: leave `Probeable` false. The property skips probing and is applied
unconditionally, and the task is `ProbePartial` with a caveat naming the family.
unconditionally, and the task is `ProbePartial` with a caveat naming the family. No family declared
today is in this state, so `runUnprobedSet`/`runUnprobedUnset` have no live caller - they are the
other half of the contract, kept for the next plugin that takes a family it does not report.

Every family also declares `Scopes`, the same `app`/`global` vocabulary an enumerable property states
by leaving a `PropertyKeys` half empty. A mapped property has an entry to say it with and a dynamic
one does not, so the family says it instead, and `validateProperty` holds a member to it: traefik's
`traefik:set` refuses a `dns-provider-*` key outside `--global`, so an app-scoped one is rejected with
the same sentence a mapped global-only property gets rather than probing a row that cannot exist.

Mark the family `Sensitive` whenever its values are credentials, whichever of those two it is.
Whether a value is a secret and whether docket can read it back are separate questions, and
Expand Down
15 changes: 13 additions & 2 deletions generate/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ func propertiesSection(schema *tasks.PropertySchema) string {
}

for _, family := range schema.Dynamic {
b.WriteString(fmt.Sprintf("\nNames starting with `%s` are also accepted. dokku validates them through `%s` rather than through its report schema, so they cannot be listed above. ",
family.Prefix, schema.Subcommand))
b.WriteString(fmt.Sprintf("\nNames starting with `%s` are also accepted %s. dokku validates them through `%s` rather than through its report schema, so they cannot be listed above. ",
family.Prefix, scopePhrase(family.Scopes), schema.Subcommand))
if family.Probeable {
b.WriteString("The plugin reports each one it has been given, so they probe for drift like any listed property.")
} else {
Expand All @@ -157,6 +157,17 @@ func propertiesSection(schema *tasks.PropertySchema) string {
return b.String()
}

// scopePhrase renders a dynamic family's scopes as the clause that follows
// "are also accepted": a family with one scope is spelled out as a restriction,
// since that is the part a reader has to know, and one with both reads as no
// restriction at all.
func scopePhrase(scopes []string) string {
if len(scopes) == 1 {
return fmt.Sprintf("in the %s scope only", scopes[0])
}
return "in the " + strings.Join(scopes, " and ") + " scopes"
}

// codeOrBlank renders a report key as inline code, or an empty cell when the
// property has no form in that scope.
func codeOrBlank(key string) string {
Expand Down
7 changes: 7 additions & 0 deletions tasks/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,13 @@ type DynamicPropertySchema struct {

// Sensitive is true when docket treats members as secrets.
Sensitive bool `json:"sensitive,omitempty"`

// Scopes is a non-empty subset of ["app", "global"], in that order, read
// the same way as PropertyEntrySchema.Scopes: a family listed only under
// "global" is rejected for an app at validate time, matching dokku's own
// rejection. traefik's `dns-provider-*` credentials are global-only that
// way, while letsencrypt's identically named family takes both.
Scopes []string `json:"scopes"`
}

// RejectedPropertySchema is a family of property names the task refuses,
Expand Down
21 changes: 16 additions & 5 deletions tasks/catalog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,19 +436,30 @@ func TestCatalogPropertySchemaSpotChecks(t *testing.T) {
if letsencrypt == nil {
t.Fatal("dokku_letsencrypt_property has no property schema")
}
want := []DynamicPropertySchema{{Prefix: "dns-provider-", Probeable: true, Sensitive: true}}
want := []DynamicPropertySchema{{
Prefix: "dns-provider-",
Probeable: true,
Sensitive: true,
Scopes: []string{PropertyScopeApp, PropertyScopeGlobal},
}}
if !reflect.DeepEqual(letsencrypt.Dynamic, want) {
t.Errorf("letsencrypt dynamic = %+v; want %+v", letsencrypt.Dynamic, want)
}

// traefik holds the same credentials, but the plugin does not report them,
// so a recipe using the family never converges - and the catalog has to say
// so without implying the values are any less secret (#457).
// traefik holds the same credentials and reports them the same way as of
// dokku 0.38.27 (#450), but `traefik:set` refuses the family outside
// --global, so the catalog has to publish it as global-only or a consumer
// validating offline accepts a recipe dokku will reject (#457, #458).
traefik := schemaFor(t, catalog, "dokku_traefik_property").PropertySchema
if traefik == nil {
t.Fatal("dokku_traefik_property has no property schema")
}
want = []DynamicPropertySchema{{Prefix: "dns-provider-", Probeable: false, Sensitive: true}}
want = []DynamicPropertySchema{{
Prefix: "dns-provider-",
Probeable: true,
Sensitive: true,
Scopes: []string{PropertyScopeGlobal},
}}
if !reflect.DeepEqual(traefik.Dynamic, want) {
t.Errorf("traefik dynamic = %+v; want %+v", traefik.Dynamic, want)
}
Expand Down
127 changes: 127 additions & 0 deletions tasks/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,133 @@ func TestExportGlobalTraefikPasswordLiftedAsSensitiveInput(t *testing.T) {
}
}

// TestExportGlobalTraefikDynamicProperties covers the export half of #450:
// dokku 0.38.27 reports every set `dns-provider-<KEY>` credential as a `global-`
// row, so the exporter lifts them straight out of the payload instead of
// dropping the whole family.
func TestExportGlobalTraefikDynamicProperties(t *testing.T) {
t.Parallel()
ctx := subprocess.ContextWithRunner(testCtx(), fakeDokku(map[string]string{
"--quiet traefik:report --global --format json": `{"global-log-level":"","global-dns-provider":"cloudflare","global-dns-provider-CLOUDFLARE_API_TOKEN":"globaltoken"}`,
}))

bodies, err := exportGlobalProperties(ctx, TraefikPropertyTask{}, func(property, value string) interface{} {
return TraefikPropertyTask{Global: true, Property: property, Value: value}
})
if err != nil {
t.Fatalf("exportGlobalProperties: %v", err)
}
got := map[string]string{}
for _, b := range bodies {
p := b.(TraefikPropertyTask)
if !p.Global {
t.Errorf("expected Global:true for %q", p.Property)
}
got[p.Property] = p.Value
}
if got["dns-provider-CLOUDFLARE_API_TOKEN"] != "globaltoken" {
t.Errorf("dns-provider-CLOUDFLARE_API_TOKEN = %q, want globaltoken", got["dns-provider-CLOUDFLARE_API_TOKEN"])
}
if got["dns-provider"] != "cloudflare" {
t.Errorf("mapped properties should still export, got %v", got)
}
if _, ok := got["log-level"]; ok {
t.Error("an empty global property must be skipped")
}
}

// TestExportTraefikDynamicPropertiesSkipAppScope pins the global-only half. A
// traefik report is global state whichever scope it is asked for, so the app
// payload carries the same `global-dns-provider-*` rows; none of them are the
// app's own value, and the family synthesizes no per-app key to lift them with.
func TestExportTraefikDynamicPropertiesSkipAppScope(t *testing.T) {
t.Parallel()
ctx := subprocess.ContextWithRunner(testCtx(), fakeDokku(map[string]string{
"--quiet traefik:report web --format json": `{"global-dns-provider-CLOUDFLARE_API_TOKEN":"globaltoken","computed-dns-provider":"cloudflare"}`,
}))

bodies, err := exportProperties(ctx, TraefikPropertyTask{}, "web", func(app, property, value string) interface{} {
return TraefikPropertyTask{App: app, Property: property, Value: value}
})
if err != nil {
t.Fatalf("exportProperties: %v", err)
}
if len(bodies) != 0 {
t.Errorf("a global-only family must export nothing per app, got %+v", bodies)
}
}

// TestExportGlobalTraefikCredentialLiftedAsSensitiveInput proves the newly
// exported credential never lands in the recipe in cleartext. Unlike the
// letsencrypt property task, this one is not built from
// SensitivePropertyFields, so nothing but the family's Sensitive mark says the
// value is a secret - which is exactly the coupling #457 broke.
func TestExportGlobalTraefikCredentialLiftedAsSensitiveInput(t *testing.T) {
t.Parallel()
ctx := subprocess.ContextWithRunner(testCtx(), fakeDokku(map[string]string{
"--quiet apps:list": "",
"--quiet traefik:report --global --format json": `{"global-dns-provider-CLOUDFLARE_API_TOKEN":"cf-s3cr3t"}`,
}))

res, err := ExportRecipe(ctx, ExportOptions{})
if err != nil {
t.Fatalf("ExportRecipe: %v", err)
}
if got := res.Vars["global_dns_provider_CLOUDFLARE_API_TOKEN"]; got != "cf-s3cr3t" {
t.Errorf("vars[global_dns_provider_CLOUDFLARE_API_TOKEN] = %q, want the credential lifted (%v)", got, res.Vars)
}
recipe, _ := res.MarshalRecipe("yaml")
out := string(recipe)
if strings.Contains(out, "cf-s3cr3t") {
t.Errorf("recipe leaked the traefik dns provider credential:\n%s", out)
}
for _, want := range []string{
"dokku_traefik_property",
"dns-provider-CLOUDFLARE_API_TOKEN",
"{{ .global_dns_provider_CLOUDFLARE_API_TOKEN }}",
"sensitive: true",
} {
if !strings.Contains(out, want) {
t.Errorf("recipe missing %q:\n%s", want, out)
}
}
}

// TestExportGlobalDynamicCredentialsCollideDeterministically covers a case only
// reachable now that both families export: the same provider env var name set
// under letsencrypt and under traefik wants the same input name. Export order
// is fixed, so the second one is suffixed rather than overwriting the first, and
// both values survive.
func TestExportGlobalDynamicCredentialsCollideDeterministically(t *testing.T) {
t.Parallel()
ctx := subprocess.ContextWithRunner(testCtx(), fakeDokku(map[string]string{
"--quiet apps:list": "",
"--quiet letsencrypt:report --global --format json": `{"global-dns-provider-CLOUDFLARE_API_TOKEN":"le-token"}`,
"--quiet traefik:report --global --format json": `{"global-dns-provider-CLOUDFLARE_API_TOKEN":"traefik-token"}`,
}))

res, err := ExportRecipe(ctx, ExportOptions{})
if err != nil {
t.Fatalf("ExportRecipe: %v", err)
}
values := map[string]bool{}
for _, value := range res.Vars {
values[value] = true
}
for _, want := range []string{"le-token", "traefik-token"} {
if !values[want] {
t.Errorf("expected %q lifted into its own input, got %v", want, res.Vars)
}
}
recipe, _ := res.MarshalRecipe("yaml")
out := string(recipe)
for _, unwanted := range []string{"le-token", "traefik-token"} {
if strings.Contains(out, unwanted) {
t.Errorf("recipe leaked %q:\n%s", unwanted, out)
}
}
}

func TestExportGlobalCertDisabledEmitsNoTask(t *testing.T) {
t.Parallel()
ctx := subprocess.ContextWithRunner(testCtx(), fakeDokku(map[string]string{
Expand Down
Loading