Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[bug] non-string environment variables can't be parsed using new config stuff #621

Closed
tsmethurst opened this issue May 31, 2022 · 2 comments
Closed

[bug] non-string environment variables can't be parsed using new config stuff #621

tsmethurst opened this issue May 31, 2022 · 2 comments
Labels
bug Something isn't working config Something needs to be made configurable, or there's a config issue
Milestone
v0.3.5 Speedy Sloth

Comments

@tsmethurst
Copy link
Contributor

When setting environment variable config through docker, non-string fields can't be parsed correctly, leading to panics:

gotosocial    | panic: 7 error(s) decoding:
gotosocial    | 
gotosocial    | * 'accounts-approval-required' expected type 'bool', got unconvertible type 'string', value: 'true'
gotosocial    | * 'accounts-reason-required' expected type 'bool', got unconvertible type 'string', value: 'false'
gotosocial    | * 'accounts-registration-open' expected type 'bool', got unconvertible type 'string', value: 'false'
gotosocial    | * 'letsencrypt-enabled' expected type 'bool', got unconvertible type 'string', value: 'false'
gotosocial    | * 'smtp-port' expected type 'int', got unconvertible type 'string', value: '587'
gotosocial    | * 'statuses-poll-max-options' expected type 'int', got unconvertible type 'string', value: '10'
gotosocial    | * 'statuses-poll-option-max-chars' expected type 'int', got unconvertible type 'string', value: '100'
gotosocial    | 
gotosocial    | goroutine 1 [running]:
gotosocial    | github.com/superseriousbusiness/gotosocial/internal/config.(*ConfigState).reloadFromViper(0xc000097880)
gotosocial    | 	/drone/src/internal/config/state.go:134 +0x53
gotosocial    | github.com/superseriousbusiness/gotosocial/internal/config.(*ConfigState).Viper.func1()
gotosocial    | 	/drone/src/internal/config/state.go:77 +0x25
gotosocial    | github.com/superseriousbusiness/gotosocial/internal/config.(*ConfigState).Viper(0x1d99180?, 0xc0003d58c0?)
gotosocial    | 	/drone/src/internal/config/state.go:81 +0xa5
gotosocial    | github.com/superseriousbusiness/gotosocial/internal/config.(*ConfigState).LoadEarlyFlags(0x4?, 0x206dcb0?)
gotosocial    | 	/drone/src/internal/config/state.go:89 +0xb9
gotosocial    | github.com/superseriousbusiness/gotosocial/internal/config.LoadEarlyFlags(...)
gotosocial    | 	/drone/src/internal/config/global.go:47
gotosocial    | main.main.func1(0xc000476780?, {0x1fbbc5b?, 0x0?, 0x0?})
gotosocial    | 	/drone/src/cmd/gotosocial/main.go:52 +0x26
gotosocial    | github.com/spf13/cobra.(*Command).execute(0xc000476780, {0x34c1068, 0x0, 0x0})
gotosocial    | 	/drone/src/vendor/github.com/spf13/cobra/command.go:835 +0x58b
gotosocial    | github.com/spf13/cobra.(*Command).ExecuteC(0xc000476500)
gotosocial    | 	/drone/src/vendor/github.com/spf13/cobra/command.go:974 +0x3b4
gotosocial    | github.com/spf13/cobra.(*Command).Execute(...)
gotosocial    | 	/drone/src/vendor/github.com/spf13/cobra/command.go:902
gotosocial    | main.main()
gotosocial    | 	/drone/src/cmd/gotosocial/main.go:68 +0x194

It looks like we need to do some conversion and parsing; from GtS's perspective, all those environment variable settings come in as strings, so for non string values we need to try to coerce them into appropriate types
@tsmethurst tsmethurst added bug Something isn't working config Something needs to be made configurable, or there's a config issue labels May 31, 2022
@tsmethurst tsmethurst added this to the v0.3.5 Speedy Sloth milestone May 31, 2022
@NyaaaWhatsUpDoc NyaaaWhatsUpDoc linked a pull request Jun 1, 2022 that will close this issue
[bugfix] #621: accept weakly typed input on mapstructure decode + add test script #622
Closed
@NyaaaWhatsUpDoc
Copy link
Member

I've added a PR for this here: #622

But even running this locally before implementing my fix I couldn't seem to trigger this panic 🤔

tsmethurst added a commit that referenced this issue Jun 8, 2022
@NyaaaWhatsUpDoc @tsmethurst
[bugfix] #621: add weak type handing to mapstructure decode (#625)
91c0ed8 
* Drone sig (#623)

* accept weakly typed input on mapstructure decode i.e. .UnmarshalMap()

Signed-off-by: kim <grufwub@gmail.com>

* add envparsing script to test for panics during environment variable parsing

Signed-off-by: kim <grufwub@gmail.com>

* add envparsing.sh script to drone commands

Signed-off-by: kim <grufwub@gmail.com>

* update drone signature

Co-authored-by: kim <grufwub@gmail.com>

* compare expected with output

* update expected output of envparsing

* update expected output to correct value

* use viper's unmarshal function instead
There were problems with marshalling
string slices from viper into the st.config
struct with the other function. Now, we
can use viper's unmarshal function and pass
in the custom decoder config that we need
as a hook. This ensures that we marshal
string slices from viper into our config
struct correctly.

Co-authored-by: tobi <31960611+tsmethurst@users.noreply.github.com>
Co-authored-by: tsmethurst <tobi.smethurst@protonmail.com>
@tsmethurst
Copy link
Contributor Author

Closed by #625

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working config Something needs to be made configurable, or there's a config issue
Projects
None yet
Milestone
v0.3.5 Speedy Sloth
2 participants
@tsmethurst @NyaaaWhatsUpDoc