Ignore unknown keys in appservice registration files

[bodqhrohro]

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.

[claudemuller]

Hey @bodqhrohro - mind if I take a swing?

[bodqhrohro]

You're welcome :)

[tichnas]

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?

[bodqhrohro]

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

[MadLittleMods]

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.

[irth]

Hi,

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

dendrite/setup/config/config_appservice.go

Line 183 in 3c419be

if err = yaml.UnmarshalStrict(configData, &appservice); err != nil {

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?

[keremgocen]

@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.

[alam0rt]

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.

[irth]

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