piotrmurach/tty-config

TTY Config doesn't follow aliases in yaml

Closed this issue ยท 6 comments

Describe the problem

TTY:Config defaults to safe_load for loading YAML when using new versions of ruby that support it. But does not give any options to make sure aliases are followed. This results in an exception when using a configuration that uses aliases.

Steps to reproduce the problem

Create a config yaml file with aliases

default: &DEFAULT
  app: myapp
  account_id: "12345567"
dev:
    <<: *DEFAULT
config = TTY::Config.new
config.filename = 'config.yml'
config.append_path Dir.pwd
config.read

Actual behaviour

Psych::BadAlias (Unknown alias: DEFAULT)

Expected behaviour

Follow aliases

Describe your environment

  • OS version: mac/linux
  • Ruby version: 2.5.5
  • TTY::Config version: 0.3.2

Ideally, could have an option that would allow the marshaller to follow aliases.

https://github.com/piotrmurach/tty-config/blob/v0.4.0/lib/tty/config/marshallers/yaml_marshaller.rb#L21

Something more like this to allow someone to override the default behavior.

follow_aliases = options[:follow_aliases] || false 
YAML.safe_load(content, aliases: follow_aliases) 

I'll try to put up a PR however I already have an alternate working solution. But wanted to mention it just in case others run into this issue.

Hi Phillip ๐Ÿ‘‹

Thanks for using tty-config and submitting this issue.

Btw, there is a newer version 0.4.0 and I'm working on 0.5.0 to be released soon.

Not sure if you noticed, you can replace the default YAML marshaller with a custom one:

class CustomYAMLMarshaller < TTY::Config::Marshallers::YAMLMarshaller
  def unmarshal(content)
    YAML.safe_load(content, aliases: true)
  end
end

config.register_marshaller :yaml, CustomYAMLMarshaller

As for adding the ability to configure the YAML marshaller, I'm not sure what is the best way to do this and whether there should be one. Thinking out loud here. The Psych.safe_load provides many options like :permitted_classes, :permitted_symbols, :aliases, :filename and I wonder where to add these keywords to make for good api? And please submit PR with any suggestions!

Curious how you have chosen to handle this yourself? Mind sharing?

Great! I did not notice that I could register my own marshaller. With that ability, I think you are right to leave it simple. I basically copied some generic config loader off stack overflow and used that. But I am gonna go back and review using a custom marshaller. Closing this issue. Thanks again for an awesome set of tools.

Only change I had to make was related to the extensions.

class CustomYAMLMarshaller < TTY::Config::Marshallers::YAMLMarshaller
  extension ".yaml", ".yml"
  def unmarshal(content)
    YAML.safe_load(content, aliases: true)    
  end
end

Thanks again! And thanks for the great tools!

Great to hear that this is working for you.

The extension not working is a bug - the variable isn't copied to the subclass. I'll fix it.

If you find any other issues or have ideas for improvement please open an issue so I can resolve them before the v0.5.0 release which I hope to cut soon.

@bluengreen You shouldn't need to use extension any more, can you please check against the master branch?

@piotrmurach will do thanks!