Ignore unknown keys in appservice registration files

[matrixbot]

This issue was originally created by @bodqhrohro at matrix-org/dendrite#1567.

Background information

• Dendrite version or git SHA: 0.2.1
• Monolith or Polylith?: Monolith
• SQLite3 or Postgres?: SQlite3
• Running in Docker?: no
• go version: 1.15

Description

Steps to reproduce

• generate an Appservice config with matrix-appservice-node
• provide that config in Dendrite config
• run Dendrite

Which results in an error:

FATA[0000] Invalid config file: yaml: unmarshal errors:
  line 16: field de.sorunome.msc2409.push_ephemeral not found in type config.ApplicationService 

because the parser doesn't ignore unknown keys.

[matrixbot]

This comment was originally posted by @claudemuller at matrix-org/dendrite#1567 (comment).

Hey @bodqhrohro - mind if I take a swing?

[matrixbot]

This comment was originally posted by @bodqhrohro at matrix-org/dendrite#1567 (comment).

You're welcome :)

[matrixbot]

This comment was originally posted by @tichnas at matrix-org/dendrite#1567 (comment).

Hi @claudemuller, are you still working on the issue?
If not, @bodqhrohro I'd love to work on this so can I please be assigned?

[matrixbot]

This comment was originally posted by @bodqhrohro at matrix-org/dendrite#1567 (comment).

I don't assign anyone. Feel free to make a PR, then reviewers will decide whose solution is better ;)

[matrixbot]

This comment was originally posted by @MadLittleMods at matrix-org/dendrite#1567 (comment).

Related to matrix-org/matrix-appservice-node#33 (fixed via matrix-org/matrix-appservice-node#34) which is now fixed so matrix-appservice-node will no longer set de.sorunome.msc2409.push_ephemeral as undefined

Might be good to fix this up so future undefined properties don't trip up the yaml parser here though.

[matrixbot]

This comment was originally posted by @irth at matrix-org/dendrite#1567 (comment).

Hi,

the error originates here, when the registration file is unmarshaled:

https://github.com/matrix-org/dendrite/blob/3c419be6af2938170d2c04f56d8616c0129ca673/setup/config/config_appservice.go#L183

The only difference between Unmarshal and UnmarshallStrict is that UnmarshallStrict does not allow unknown keys and duplicate keys. (docs)

Replacing this UnmarshalStrict with Unmarshal would fix this issue.

However, this brings another issue into mind - do we want to verify that the keys that we require are actually present? Or, are we doing that somewhere else?

[matrixbot]

This comment was originally posted by @keremgocen at matrix-org/dendrite#1567 (comment).

@irth was there a conclusion about your comment? I'm happy to send a PR with that small change if that's still the solution you guys think is the best.

[matrixbot]

This comment was originally posted by @alam0rt at matrix-org/dendrite#1567 (comment).

I was going to write up a simple PR to "fix" this, but personally UnmarshallStrict feels like a better user experience - i.e. if I had a typo in my config I'd prefer to know at start up rather than spending ages working out why my config was being silently swallowed.

[matrixbot]

This comment was originally posted by @irth at matrix-org/dendrite#1567 (comment).

Perhaps we might need to Unmarshal it to something less typed - map[string]interface{}, and then do the verification and fill the struct manually.