cfer icon indicating copy to clipboard operation
cfer copied to clipboard

Add explicit export option with simple string argument

Open rlister opened this issue 7 years ago • 5 comments

Fixes #55

Generic options hash in output function allowed the following un-rubyish construct:

output :OutputName, Fn::ref(:ResourceName), 
  Description: 'The resource that does stuff',
  Export: {'Name' => Fn::sub('${AWS::StackName}-OutputName')}

This change allows lower-case keys and a simpler string argument for export:

output :OutputName, Fn::ref(:ResourceName), 
  description: 'The resource that does stuff',
  export: Fn::sub('${AWS::StackName}-OutputName')

rlister avatar Mar 24 '18 14:03 rlister

I can't accept this one as-is because it breaks the Export functionality as it works today. I know of at least two companies that are using that today, and I'm not comfortable asking them to make this change for stylistic reasons.

"publiczone": {
  "Export": {
    "Name": {
      "Name": "PublicZone"
      }
    }
  },
  "Value": {
    "Ref": "PublicZone"
  }
},

However, I do appreciate that the current way of using exports is not the most rubyish.

The way I'd prefer to accomplish this is something like this:

def output(name, value, options = {})
  self[:Outputs][name] = options.merge('Value' => value)
end

def export(name, value, options = {})
    output name, value, options.merge(Export: { 'Name' => name })
end

That way, output is always a fall-back escape-hatch.

seanedwards avatar Apr 23 '18 18:04 seanedwards

Here is my latest technique for solving this problem. This method also includes a naming convention, based on our "Environment+Application+Facet" model for deploying the same infrastructure multiple times. Solving the "Export" noise hasn't been a high priority for me because I usually use it encapsulated by a specific pattern like this, since exports are a global namespace outside of stacks.

Cfer::Core::Stack.extend_stack do
  def exported_val_name(name, options={})
    env = options[:environment] || parameters[:environment] # parameters[:environment] is always present
    app = options[:application] || parameters[:application] # parameters[:application] is always present
    facet = options[:facet] || parameters[:facet] # parameters[:facet] is always present, and defaults to "main"
    { "Fn::Join": [options[:delim] || "-", [env, app, facet, name]] }
  end

  def opscues_import(app, rc, options={})
    val_name = exported_val_name(rc, options.merge(application: app))

    { "Fn::ImportValue": val_name }
  end

  def opscues_export(name, value, options = {})
    export_name = exported_val_name(name, options)
    output name.to_s.gsub(/[^A-Za-z0-9]+/, ''), value, Export: { Name: export_name }
  end
end

seanedwards avatar Apr 23 '18 18:04 seanedwards

@seanedwards Sure, I understand the motivation here. I'm not super keen on an explicit export function: my main reason for liking cfer in the first place was that it is such a thin layer on cloudformation. One can read cloudformation examples in json or yaml, and taking a guess at how that should look in ruby generally works.

How would you feel about an [Ee]xport option to the existing output function, that checks the class of its argument (Hash vs String) and does the right thing?

rlister avatar Apr 23 '18 18:04 rlister

So if I'm understanding: output :Test, "Value", export: "TestValue" <- This automatically wraps the value in { Name: "Value" } because it is a string, not a hash.

That sounds good to me

seanedwards avatar Apr 23 '18 18:04 seanedwards

@seanedwards Correct, both forms should work. I'll have a hack on it in the next couple of days as dayjob+life permits.

rlister avatar Apr 23 '18 19:04 rlister