You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by ma...@redhat.com on 2011/05/16 09:06:03 UTC

[PATCH] implements 'firewalls' - which are ec2 security groups

From: marios <ma...@redhat.com>


Signed-off-by: marios <ma...@redhat.com>
---
 server/deltacloud.rb                             |    2 +
 server/lib/deltacloud/base_driver/base_driver.rb |    6 +-
 server/lib/deltacloud/base_driver/features.rb    |    8 +-
 server/lib/deltacloud/drivers/ec2/ec2_driver.rb  |  142 +++++++++++++++++++++-
 server/lib/deltacloud/models/firewall.rb         |   22 ++++
 server/lib/deltacloud/models/firewall_rule.rb    |   23 ++++
 server/server.rb                                 |  105 ++++++++++++++++
 server/views/firewalls/index.html.haml           |   25 ++++
 server/views/firewalls/index.xml.haml            |   23 ++++
 server/views/firewalls/new.html.haml             |   11 ++
 server/views/firewalls/show.html.haml            |   41 ++++++
 server/views/firewalls/show.xml.haml             |   21 +++
 12 files changed, 421 insertions(+), 8 deletions(-)
 create mode 100644 server/lib/deltacloud/models/firewall.rb
 create mode 100644 server/lib/deltacloud/models/firewall_rule.rb
 create mode 100644 server/views/firewalls/index.html.haml
 create mode 100644 server/views/firewalls/index.xml.haml
 create mode 100644 server/views/firewalls/new.html.haml
 create mode 100644 server/views/firewalls/show.html.haml
 create mode 100644 server/views/firewalls/show.xml.haml

diff --git a/server/deltacloud.rb b/server/deltacloud.rb
index 7caf34f..5628e31 100644
--- a/server/deltacloud.rb
+++ b/server/deltacloud.rb
@@ -36,6 +36,8 @@ require 'deltacloud/models/storage_volume'
 require 'deltacloud/models/bucket'
 require 'deltacloud/models/blob'
 require 'deltacloud/models/load_balancer'
+require 'deltacloud/models/firewall'
+require 'deltacloud/models/firewall_rule'
 
 require 'deltacloud/validation'
 require 'deltacloud/helpers'
diff --git a/server/lib/deltacloud/base_driver/base_driver.rb b/server/lib/deltacloud/base_driver/base_driver.rb
index d9ebd92..06363d9 100644
--- a/server/lib/deltacloud/base_driver/base_driver.rb
+++ b/server/lib/deltacloud/base_driver/base_driver.rb
@@ -183,8 +183,12 @@ module Deltacloud
       keys(credentials, opts).first if has_capability?(:keys)
     end
 
+    def firewall(credentials, opts={})
+      firewalls(credentials, opts).first if has_capability?(:firewalls)
+    end
+
     MEMBER_SHOW_METHODS =
-      [ :realm, :image, :instance, :storage_volume, :bucket, :blob, :key ]
+      [ :realm, :image, :instance, :storage_volume, :bucket, :blob, :key, :firewall ]
     
     def has_capability?(capability)
       if MEMBER_SHOW_METHODS.include?(capability.to_sym)
diff --git a/server/lib/deltacloud/base_driver/features.rb b/server/lib/deltacloud/base_driver/features.rb
index 65c4cba..cb25a3b 100644
--- a/server/lib/deltacloud/base_driver/features.rb
+++ b/server/lib/deltacloud/base_driver/features.rb
@@ -183,11 +183,11 @@ module Deltacloud
       end
     end
 
-    declare_feature :instances, :security_group do
-      description "Put instance in one or more security groups on launch"
+    declare_feature :instances, :firewall do
+      description "Put instance in one or more firewalls (security groups) on launch"
       operation :create do
-        param :security_group, :array, :optional, nil,
-        "Array of security group names"
+        param :firewalls, :array, :optional, nil,
+        "Array of firewall (security group) id"
       end
     end
 
diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
index 14c5829..4ef6efd 100644
--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
@@ -16,6 +16,7 @@
 
 require 'deltacloud/base_driver'
 require 'aws'
+require 'base64'
 
 class Instance
   attr_accessor :keyname
@@ -33,12 +34,13 @@ module Deltacloud
       class EC2Driver < Deltacloud::BaseDriver
 
         def supported_collections
-          DEFAULT_COLLECTIONS + [ :keys, :buckets, :load_balancers, :addresses ]
+
+          DEFAULT_COLLECTIONS + [ :keys, :buckets, :load_balancers, :addresses, :firewalls ]
         end
 
         feature :instances, :user_data
         feature :instances, :authentication_key
-        feature :instances, :security_group
+        feature :instances, :firewall
         feature :instances, :instance_count
         feature :images, :owner_id
         feature :buckets, :bucket_location
@@ -201,7 +203,7 @@ module Deltacloud
           instance_options.merge!(:key_name => opts[:keyname]) if opts[:keyname]
           instance_options.merge!(:availability_zone => opts[:realm_id]) if opts[:realm_id]
           instance_options.merge!(:instance_type => opts[:hwp_id]) if opts[:hwp_id] && opts[:hwp_id].length > 0
-          instance_options.merge!(:group_ids => opts[:security_group]) if opts[:security_group]
+          instance_options.merge!(:group_ids => opts[:firewall]) if opts[:firewall]
           instance_options.merge!(
             :min_count => opts[:instance_count],
             :max_count => opts[:instance_count]
@@ -516,6 +518,7 @@ module Deltacloud
           end
         end
 
+
         def addresses(credentials, opts={})
           ec2 = new_client(credentials)
           address_id = (opts and opts[:id]) ? [opts[:id]] : []
@@ -571,6 +574,63 @@ module Deltacloud
           end
         end
 
+#--
+#FIREWALLS - ec2 security groups
+#--
+			def firewalls(credentials, opts={})
+          ec2 = new_client(credentials)
+          the_firewalls = []
+          groups = []
+          if opts[:id]
+            groups = ec2.describe_security_groups([opts[:id]])
+          else
+            groups = ec2.describe_security_groups()
+          end
+          groups.each do |security_group|
+            the_firewalls << convert_security_group(security_group)
+          end
+          filter_on(the_firewalls, :id, opts)
+		  end
+
+#--
+#Create firewall
+#--
+        def create_firewall(credentials, opts={})
+          ec2 = new_client(credentials)
+          ec2.create_security_group(opts["name"], opts["description"])
+          Firewall.new( { :id=>opts["name"], :name=>opts["name"],
+                          :description => opts["description"], :owner_id => "", :rules => [] } )
+        end
+
+#--
+#Delete firewall
+#--
+        def delete_firewall(credentials, opts={})
+          ec2 = new_client(credentials)
+          ec2.delete_security_group(opts["id"])
+        end
+#--
+#Create firewall rule
+#--
+        def create_firewall_rule(credentials, opts={})
+          ec2 = new_client(credentials)
+          groups = []
+          opts['groups'].each do |k,v|
+            groups << {"group_name" => k, "owner" =>v}
+          end
+          ec2.manage_security_group_ingress(opts['firewall'], opts['from_port'], opts['to_port'], opts['protocol'],
+              "authorize", opts['addresses'], groups)
+        end
+#--
+#Delete firewall rule
+#--
+        def delete_firewall_rule(credentials, opts={})
+          ec2 = new_client(credentials)
+          firewall = opts[:firewall]
+          protocol, from_port, to_port, addresses, groups = firewall_rule_params(opts[:rule_id])
+          ec2.manage_security_group_ingress(firewall, from_port, to_port, protocol, "revoke", addresses, groups)
+        end
+
         def valid_credentials?(credentials)
           retval = true
           begin
@@ -764,6 +824,82 @@ module Deltacloud
           balancer
         end
 
+        #generate uid from firewall rule parameters (amazon doesn't do this for us
+        def firewall_rule_id(user_id, protocol, from_port, to_port, sources)
+          sources_string = ""
+          sources.each do |source|
+            source.each_pair do |key,value|
+              sources_string<< "#{key}=#{value}&"
+            end
+            sources_string.chomp!("&")
+            sources_string<<"|"
+          end
+          sources_string.chomp!("|")
+          #"type=group&owner=123456789012&name=new_firewall|type=address&family=ipv4&address=192.168.1.1&prefix=24"
+          id_string = "user #{user_id}:::protocol #{protocol}:::from_port #{from_port}:::to_port #{to_port}:::sources #{sources_string}"
+          Base64.encode64(id_string)
+        end
+
+        #extract params from uid
+        def firewall_rule_params(id)
+          param_string = Base64.decode64(id)
+          # "#{user_id}:::#{protocol}:::#{from_port}:::#{to_port}:::#{sources_string}"
+          params = param_string.split(":::")
+          protocol = params.grep(/protocol/).first.split(" ").last
+          from_port = params.grep(/from_port/).first.split(" ").last
+          to_port = params.grep(/to_port/).first.split(" ").last
+          sources = params.grep(/sources/).first.split(" ").last
+          addresses = []
+          groups = []
+          sources.split("|").each do |source|
+            current = source.split("&")
+            type = current.grep(/type/).first.split("=").last
+            case type
+              when 'group'
+                #type=group&owner=123456789012&name=default
+                name = current.grep(/name/).first.split("=").last
+                owner = current.grep(/owner/).first.split("=").last
+                groups << {'group_name' => name, 'owner' => owner}
+              when 'address'
+                #type=address&family=ipv4&address=10.1.1.1&prefix=24
+                address = current.grep(/address/).last.split("=").last
+                address<<"/#{current.grep(/prefix/).first.split("=").last}"
+                addresses << address
+            end
+          end
+          return protocol, from_port, to_port, addresses, groups
+        end
+
+        #Convert ec2 security group to server/lib/deltacloud/models/firewall
+        def convert_security_group(security_group)
+          rules = []
+          security_group[:aws_perms].each do |perm|
+            sources = []
+            perm[:groups].each do |group|
+              sources << {:type => "group", :name => group[:group_name], :owner => group[:owner]}
+            end
+            perm[:ip_ranges].each do |ip|
+              sources << {:type => "address", :family=>"ipv4",
+                          :address=>ip[:cidr_ip].split("/").first,
+                          :prefix=>ip[:cidr_ip].split("/").last}
+            end
+            rule_id = firewall_rule_id(security_group[:aws_owner], perm[:protocol],
+                                       perm[:from_port] , perm[:to_port], sources)
+            rules << Firewall_Rule.new({:id => rule_id,
+                                        :allow_protocol => perm[:protocol],
+                                        :port_from => perm[:from_port],
+                                        :port_to => perm[:to_port],
+                                        :direction => 'ingress',
+                                        :sources => sources})
+          end
+          Firewall.new(  {  :id => security_group[:aws_group_name],
+                            :name => security_group[:aws_group_name],
+                            :description => security_group[:aws_description],
+                            :owner_id => security_group[:aws_owner],
+                            :rules => rules
+                      }  )
+        end
+
         def convert_state(ec2_state)
           case ec2_state
             when "terminated"
diff --git a/server/lib/deltacloud/models/firewall.rb b/server/lib/deltacloud/models/firewall.rb
new file mode 100644
index 0000000..dc0ae3d
--- /dev/null
+++ b/server/lib/deltacloud/models/firewall.rb
@@ -0,0 +1,22 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#       http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+class Firewall < BaseModel
+  attr_accessor :name
+  attr_accessor :description
+  attr_accessor :owner_id
+  attr_accessor :rules
+end
\ No newline at end of file
diff --git a/server/lib/deltacloud/models/firewall_rule.rb b/server/lib/deltacloud/models/firewall_rule.rb
new file mode 100644
index 0000000..df353a5
--- /dev/null
+++ b/server/lib/deltacloud/models/firewall_rule.rb
@@ -0,0 +1,23 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.  The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License.  You may obtain a copy of the License at
+#
+#       http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+class Firewall_Rule < BaseModel
+  attr_accessor :allow_protocol # tcp/udp/icmp
+  attr_accessor :port_from
+  attr_accessor :port_to
+  attr_accessor :sources
+  attr_accessor :direction #ingress egress
+end
diff --git a/server/server.rb b/server/server.rb
index 4da8357..a318df6 100644
--- a/server/server.rb
+++ b/server/server.rb
@@ -843,6 +843,7 @@ collection :buckets do
 
 end
 
+
 get '/api/addresses/:id/associate' do
   @instances = driver.instances(credentials)
   @address = Address::new(:id => params[:id])
@@ -929,3 +930,107 @@ collection :addresses do
   end
 
 end
+
+#html for creating a new firewall
+get '/api/firewalls/new' do
+  respond_to do |format|
+    format.html { haml :"firewalls/new" }
+  end
+end
+
+#html for creating a new firewall rule
+get '/api/firewalls/:firewall/new_rule' do
+  @firewall_name = params[:firewall]
+  respond_to do |format|
+    format.html {haml :"firewalls/new_rule" }
+  end
+end
+
+#create new firewall rule
+post '/api/firewalls/:firewall/rules' do
+  opts = {}
+  opts['firewall'] = params[:firewall] # must specify group_name or group_id
+  opts['protocol'] = params[:protocol]
+  opts['from_port'] = params[:from_port]
+  opts['to_port'] = params[:to_port]
+  addresses =  params.inject([]){|result,current| result << current.last unless current.grep(/^ip[-_]address/i).empty?; result}
+  groups = {}
+  max_groups  = params.select{|k,v| k=~/^group/}.size/2
+  for i in (1..max_groups) do
+    groups.merge!({params["group#{i}"]=>params["group#{i}owner"]})
+  end
+  opts['addresses'] = addresses
+  opts['groups'] = groups
+  driver.create_firewall_rule(credentials, opts)
+  @firewall = driver.firewall(credentials, {:id => params[:firewall]})
+  respond_to do |format|
+    format.html {haml :"firewalls/show"}
+    format.xml do
+      response.status = 201 #created
+      haml :"firewalls/show"
+    end
+  end
+end
+
+#delete a firewall rule
+delete '/api/firewalls/:firewall/:rule' do
+  opts = {}
+  opts[:firewall] = params[:firewall]
+  opts[:rule_id] = params[:rule]
+  driver.delete_firewall_rule(credentials, opts)
+  respond_to do |format|
+    format.html {redirect firewall_url(params[:firewall])}
+    format.xml {204}
+    format.json {204}
+  end
+end
+
+
+collection :firewalls do
+  description "Allow user to define firewall rules for an instance (ec2 security groups) eg expose ssh access [port 22, tcp]."
+
+  operation :index do
+    description 'List all firewalls'
+    with_capability :firewalls
+    control { filter_all(:firewalls) }
+  end
+
+  operation :show do
+    description 'Show details for a specific firewall - list all rules'
+    with_capability :firewall
+    param :id,            :string,    :required
+    control { show(:firewall) }
+  end
+
+  operation :create do
+    description 'Create a new firewall'
+    with_capability :create_firewall
+    param :name,          :string,    :required
+    param :description,   :string,    :required
+    control do
+      @firewall = driver.create_firewall(credentials, params )
+      respond_to do |format|
+        format.xml do
+          response.status = 201  # Created
+          haml :"firewalls/show"
+        end
+        format.html {haml :"firewalls/show"}
+      end
+    end
+  end
+
+  operation :destroy do
+    description 'Delete a specified firewall - error if firewall has rules'
+    with_capability :delete_firewall
+    param :id,            :string,    :required
+    control do
+      driver.delete_firewall(credentials, params)
+      respond_to do |format|
+        format.xml { 204 }
+        format.json {  204 }
+        format.html {  redirect(firewalls_url) }
+      end
+    end
+  end
+
+end #firewalls
diff --git a/server/views/firewalls/index.html.haml b/server/views/firewalls/index.html.haml
new file mode 100644
index 0000000..3312a32
--- /dev/null
+++ b/server/views/firewalls/index.html.haml
@@ -0,0 +1,25 @@
+%h1 Firewalls
+%br
+%p
+  =link_to 'Create new firewall', "/api/firewalls/new"
+%table.display
+  %thead
+    %tr
+      %th Id
+      %th Name
+      %th Description
+      %th Owner ID
+      %th Rules
+  %tbody
+    - @firewalls.each do |firewall|
+      %tr
+        %td
+          = link_to firewall.id, firewall_url(firewall.id)
+        %td
+          = firewall.name
+        %td
+          = firewall.description
+        %td
+          = firewall.owner_id
+        %td
+          = link_to 'view rules', firewall_url(firewall.id)
diff --git a/server/views/firewalls/index.xml.haml b/server/views/firewalls/index.xml.haml
new file mode 100644
index 0000000..8397f2d
--- /dev/null
+++ b/server/views/firewalls/index.xml.haml
@@ -0,0 +1,23 @@
+!!! XML
+%firewalls
+  - @firewalls.each do |firewall|
+    %firewall{:href => firewall_url(firewall.id), :id => firewall.id}
+      - firewall.attributes.select{ |attr| attr != :id && attr!= :rules}.each do |attribute|
+        - haml_tag("#{attribute}".tr('-', '_'), :<) do
+          - if [:name, :description].include?(attribute)
+            =cdata do
+              - haml_concat firewall.send(attribute)
+          - else
+            - haml_concat firewall.send(attribute)
+      %rules
+        - firewall.rules.each do |rule|
+          %rule
+            - rule.attributes.select{|attr| attr != :sources}.each do |rule_attrib|
+              - haml_tag("#{rule_attrib}".tr('-', '_'), :<) do
+                - haml_concat rule.send(rule_attrib)
+            %sources
+              - rule.sources.each do |source|
+                - if source[:type] == "group"
+                  %source{:name => source[:name], :type=> source[:type], :owner=> source[:owner]}
+                - else
+                  %source{:prefix => source[:prefix], :address=> source[:address], :family=>source[:family], :type => source[:type]}
\ No newline at end of file
diff --git a/server/views/firewalls/new.html.haml b/server/views/firewalls/new.html.haml
new file mode 100644
index 0000000..4a230a6
--- /dev/null
+++ b/server/views/firewalls/new.html.haml
@@ -0,0 +1,11 @@
+%h1 New Firewall
+
+%form{:action => firewalls_url, :method => :post}
+  %label
+    Firewall Name
+    %input{:name => 'name', :size => 25}/
+    %br
+  %label
+    Firewall Description
+    %input{:name => 'description', :size => 100}/
+  %input{:type => :submit, :name => "commit", :value=>"create"}
\ No newline at end of file
diff --git a/server/views/firewalls/show.html.haml b/server/views/firewalls/show.html.haml
new file mode 100644
index 0000000..e6a186f
--- /dev/null
+++ b/server/views/firewalls/show.html.haml
@@ -0,0 +1,41 @@
+%h1 Firewall
+%h2
+  = @firewall.id
+%dl
+  %di
+    %dt Name
+    %dd
+      = @firewall.name
+    %dt Owner
+    %dd
+      = @firewall.owner_id
+    %dt Description
+    %dd
+      = @firewall.description
+
+%h2
+  Rules
+  %br
+  %p
+    =link_to 'Create a new rule', "/api/firewalls/#{@firewall.name}/new_rule"
+%dl
+  - @firewall.rules.each do |rule|
+    %di
+      Rule
+      - rule.attributes.select{|attr| attr != :sources}.each do |attrib|
+        %dt #{attrib}
+        %dd
+          = rule.send(attrib)
+      %dt sources
+      %dd
+        - rule.sources.each do |source|
+          - if source[:type] == "group"
+            type: #{source[:type]}, name: #{source[:name]}, owner: #{source[:owner]}
+            %br
+          - else
+            type: #{source[:type]}, family: #{source[:family]}, address: #{source[:address]}, prefix: #{source[:prefix]}
+            %br
+      %dd
+        = link_to_action 'Delete Rule', destroy_firewall_url(@firewall.name) + "/#{rule.id}", :delete
+  %dd
+    = link_to_action 'Delete Firewall', destroy_firewall_url(@firewall.name), :delete
\ No newline at end of file
diff --git a/server/views/firewalls/show.xml.haml b/server/views/firewalls/show.xml.haml
new file mode 100644
index 0000000..200d2cd
--- /dev/null
+++ b/server/views/firewalls/show.xml.haml
@@ -0,0 +1,21 @@
+!!! XML
+%firewall{:href => firewall_url(@firewall.id), :id => @firewall.id}
+  - @firewall.attributes.select{ |attr| attr != :id && attr!= :rules}.each do |attribute|
+    - haml_tag("#{attribute}".tr('-', '_'), :<) do
+      - if [:name, :description].include?(attribute)
+        =cdata do
+          - haml_concat @firewall.send(attribute)
+      - else
+        - haml_concat @firewall.send(attribute)
+  %rules
+    - @firewall.rules.each do |rule|
+      %rule
+        - rule.attributes.select{|attr| attr != :sources}.each do |rule_attrib|
+          - haml_tag("#{rule_attrib}".tr('-', '_'), :<) do
+            - haml_concat rule.send(rule_attrib)
+        %sources
+          - rule.sources.each do |source|
+            - if source[:type] == "group"
+              %source{:name => source[:name], :type=> source[:type], :owner=>source[:owner]}
+            - else
+              %source{:prefix => source[:prefix], :address=> source[:address], :family=>source[:family], :type => source[:type]}
\ No newline at end of file
-- 
1.7.3.4


Re: [PATCH] implements 'firewalls' - which are ec2 security groups

Posted by "marios@redhat.com" <ma...@redhat.com>.
On 16/05/11 13:30, Michal Fojtik wrote:
> On May 16, 2011, at 9:06 AM, marios@redhat.com wrote:
>
> Actually NACK.
>
> However this patch works fine the only major doubt I have on it
> are missing 'safely..end' block. This prevent reporting errors
> to client correctly.
>
> It would be also nice to have firewalls collection in Mock driver if it's
> possible to run some tests on it, eventually add this stuff to EC2 Cucumber
> features (I can help you with this).
>

OK thanks for the feedback (totally forgot the safely..end stuff) - I'm 
sure there will be lots of changes (e.g. structure of xml? sanity of the 
'uid' i generate etc) so will give David and others a chance to see this 
before I re-work,

marios





>    -- Michal
>
>> From: marios<ma...@redhat.com>
>>
>>
>> Signed-off-by: marios<ma...@redhat.com>
>> ---
>> server/deltacloud.rb                             |    2 +
>> server/lib/deltacloud/base_driver/base_driver.rb |    6 +-
>> server/lib/deltacloud/base_driver/features.rb    |    8 +-
>> server/lib/deltacloud/drivers/ec2/ec2_driver.rb  |  142 +++++++++++++++++++++-
>> server/lib/deltacloud/models/firewall.rb         |   22 ++++
>> server/lib/deltacloud/models/firewall_rule.rb    |   23 ++++
>> server/server.rb                                 |  105 ++++++++++++++++
>> server/views/firewalls/index.html.haml           |   25 ++++
>> server/views/firewalls/index.xml.haml            |   23 ++++
>> server/views/firewalls/new.html.haml             |   11 ++
>> server/views/firewalls/show.html.haml            |   41 ++++++
>> server/views/firewalls/show.xml.haml             |   21 +++
>> 12 files changed, 421 insertions(+), 8 deletions(-)
>> create mode 100644 server/lib/deltacloud/models/firewall.rb
>> create mode 100644 server/lib/deltacloud/models/firewall_rule.rb
>> create mode 100644 server/views/firewalls/index.html.haml
>> create mode 100644 server/views/firewalls/index.xml.haml
>> create mode 100644 server/views/firewalls/new.html.haml
>> create mode 100644 server/views/firewalls/show.html.haml
>> create mode 100644 server/views/firewalls/show.xml.haml
>>
>> diff --git a/server/deltacloud.rb b/server/deltacloud.rb
>> index 7caf34f..5628e31 100644
>> --- a/server/deltacloud.rb
>> +++ b/server/deltacloud.rb
>> @@ -36,6 +36,8 @@ require 'deltacloud/models/storage_volume'
>> require 'deltacloud/models/bucket'
>> require 'deltacloud/models/blob'
>> require 'deltacloud/models/load_balancer'
>> +require 'deltacloud/models/firewall'
>> +require 'deltacloud/models/firewall_rule'
>>
>> require 'deltacloud/validation'
>> require 'deltacloud/helpers'
>> diff --git a/server/lib/deltacloud/base_driver/base_driver.rb b/server/lib/deltacloud/base_driver/base_driver.rb
>> index d9ebd92..06363d9 100644
>> --- a/server/lib/deltacloud/base_driver/base_driver.rb
>> +++ b/server/lib/deltacloud/base_driver/base_driver.rb
>> @@ -183,8 +183,12 @@ module Deltacloud
>>        keys(credentials, opts).first if has_capability?(:keys)
>>      end
>>
>> +    def firewall(credentials, opts={})
>> +      firewalls(credentials, opts).first if has_capability?(:firewalls)
>> +    end
>> +
>>      MEMBER_SHOW_METHODS =
>> -      [ :realm, :image, :instance, :storage_volume, :bucket, :blob, :key ]
>> +      [ :realm, :image, :instance, :storage_volume, :bucket, :blob, :key, :firewall ]
>>
>>      def has_capability?(capability)
>>        if MEMBER_SHOW_METHODS.include?(capability.to_sym)
>> diff --git a/server/lib/deltacloud/base_driver/features.rb b/server/lib/deltacloud/base_driver/features.rb
>> index 65c4cba..cb25a3b 100644
>> --- a/server/lib/deltacloud/base_driver/features.rb
>> +++ b/server/lib/deltacloud/base_driver/features.rb
>> @@ -183,11 +183,11 @@ module Deltacloud
>>        end
>>      end
>>
>> -    declare_feature :instances, :security_group do
>> -      description "Put instance in one or more security groups on launch"
>> +    declare_feature :instances, :firewall do
>> +      description "Put instance in one or more firewalls (security groups) on launch"
>>        operation :create do
>> -        param :security_group, :array, :optional, nil,
>> -        "Array of security group names"
>> +        param :firewalls, :array, :optional, nil,
>> +        "Array of firewall (security group) id"
>>        end
>>      end
>>
>> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> index 14c5829..4ef6efd 100644
>> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> @@ -16,6 +16,7 @@
>>
>> require 'deltacloud/base_driver'
>> require 'aws'
>> +require 'base64'
>>
>> class Instance
>>    attr_accessor :keyname
>> @@ -33,12 +34,13 @@ module Deltacloud
>>        class EC2Driver<  Deltacloud::BaseDriver
>>
>>          def supported_collections
>> -          DEFAULT_COLLECTIONS + [ :keys, :buckets, :load_balancers, :addresses ]
>> +
>> +          DEFAULT_COLLECTIONS + [ :keys, :buckets, :load_balancers, :addresses, :firewalls ]
>>          end
>>
>>          feature :instances, :user_data
>>          feature :instances, :authentication_key
>> -        feature :instances, :security_group
>> +        feature :instances, :firewall
>>          feature :instances, :instance_count
>>          feature :images, :owner_id
>>          feature :buckets, :bucket_location
>> @@ -201,7 +203,7 @@ module Deltacloud
>>            instance_options.merge!(:key_name =>  opts[:keyname]) if opts[:keyname]
>>            instance_options.merge!(:availability_zone =>  opts[:realm_id]) if opts[:realm_id]
>>            instance_options.merge!(:instance_type =>  opts[:hwp_id]) if opts[:hwp_id]&&  opts[:hwp_id].length>  0
>> -          instance_options.merge!(:group_ids =>  opts[:security_group]) if opts[:security_group]
>> +          instance_options.merge!(:group_ids =>  opts[:firewall]) if opts[:firewall]
>>            instance_options.merge!(
>>              :min_count =>  opts[:instance_count],
>>              :max_count =>  opts[:instance_count]
>> @@ -516,6 +518,7 @@ module Deltacloud
>>            end
>>          end
>>
>> +
>>          def addresses(credentials, opts={})
>>            ec2 = new_client(credentials)
>>            address_id = (opts and opts[:id]) ? [opts[:id]] : []
>> @@ -571,6 +574,63 @@ module Deltacloud
>>            end
>>          end
>>
>> +#--
>> +#FIREWALLS - ec2 security groups
>> +#--
>> +			def firewalls(credentials, opts={})
>
> ^^ looks like a formatting typo.
>
>> +          ec2 = new_client(credentials)
>> +          the_firewalls = []
>> +          groups = []
>> +          if opts[:id]
>> +            groups = ec2.describe_security_groups([opts[:id]])
>> +          else
>> +            groups = ec2.describe_security_groups()
>> +          end
>> +          groups.each do |security_group|
>> +            the_firewalls<<
>> +          end
>
> Could be replaced by:
>
> the_firewalls = groups.collect { |security_group| convert_security_group(security_group) }
>
> ** Also 'safely..end' blocks are missing here and on other places bellow **.
>
>
>> +          filter_on(the_firewalls, :id, opts)
>> +		  end
>> +
>> +#--
>> +#Create firewall
>> +#--
>> +        def create_firewall(credentials, opts={})
>> +          ec2 = new_client(credentials)
>> +          ec2.create_security_group(opts["name"], opts["description"])
>> +          Firewall.new( { :id=>opts["name"], :name=>opts["name"],
>> +                          :description =>  opts["description"], :owner_id =>  "", :rules =>  [] } )
>> +        end
>> +
>> +#--
>> +#Delete firewall
>> +#--
>> +        def delete_firewall(credentials, opts={})
>> +          ec2 = new_client(credentials)
>> +          ec2.delete_security_group(opts["id"])
>> +        end
>> +#--
>> +#Create firewall rule
>> +#--
>> +        def create_firewall_rule(credentials, opts={})
>> +          ec2 = new_client(credentials)
>> +          groups = []
>> +          opts['groups'].each do |k,v|
>> +            groups<<  {"group_name" =>  k, "owner" =>v}
>> +          end
>> +          ec2.manage_security_group_ingress(opts['firewall'], opts['from_port'], opts['to_port'], opts['protocol'],
>> +              "authorize", opts['addresses'], groups)
>> +        end
>> +#--
>> +#Delete firewall rule
>> +#--
>> +        def delete_firewall_rule(credentials, opts={})
>> +          ec2 = new_client(credentials)
>> +          firewall = opts[:firewall]
>> +          protocol, from_port, to_port, addresses, groups = firewall_rule_params(opts[:rule_id])
>> +          ec2.manage_security_group_ingress(firewall, from_port, to_port, protocol, "revoke", addresses, groups)
>> +        end
>> +
>>          def valid_credentials?(credentials)
>>            retval = true
>>            begin
>> @@ -764,6 +824,82 @@ module Deltacloud
>>            balancer
>>          end
>>
>> +        #generate uid from firewall rule parameters (amazon doesn't do this for us
>> +        def firewall_rule_id(user_id, protocol, from_port, to_port, sources)
>> +          sources_string = ""
>> +          sources.each do |source|
>> +            source.each_pair do |key,value|
>> +              sources_string<<  "#{key}=#{value}&"
>> +            end
>> +            sources_string.chomp!("&")
>> +            sources_string<<"|"
>> +          end
>> +          sources_string.chomp!("|")
>> +          #"type=group&owner=123456789012&name=new_firewall|type=address&family=ipv4&address=192.168.1.1&prefix=24"
>> +          id_string = "user #{user_id}:::protocol #{protocol}:::from_port #{from_port}:::to_port #{to_port}:::sources #{sources_string}"
>> +          Base64.encode64(id_string)
>> +        end
>> +
>> +        #extract params from uid
>> +        def firewall_rule_params(id)
>> +          param_string = Base64.decode64(id)
>> +          # "#{user_id}:::#{protocol}:::#{from_port}:::#{to_port}:::#{sources_string}"
>> +          params = param_string.split(":::")
>> +          protocol = params.grep(/protocol/).first.split(" ").last
>> +          from_port = params.grep(/from_port/).first.split(" ").last
>> +          to_port = params.grep(/to_port/).first.split(" ").last
>> +          sources = params.grep(/sources/).first.split(" ").last
>> +          addresses = []
>> +          groups = []
>> +          sources.split("|").each do |source|
>> +            current = source.split("&")
>> +            type = current.grep(/type/).first.split("=").last
>> +            case type
>> +              when 'group'
>> +                #type=group&owner=123456789012&name=default
>> +                name = current.grep(/name/).first.split("=").last
>> +                owner = current.grep(/owner/).first.split("=").last
>> +                groups<<  {'group_name' =>  name, 'owner' =>  owner}
>> +              when 'address'
>> +                #type=address&family=ipv4&address=10.1.1.1&prefix=24
>> +                address = current.grep(/address/).last.split("=").last
>> +                address<<"/#{current.grep(/prefix/).first.split("=").last}"
>> +                addresses<<  address
>> +            end
>> +          end
>> +          return protocol, from_port, to_port, addresses, groups
>> +        end
>> +
>> +        #Convert ec2 security group to server/lib/deltacloud/models/firewall
>> +        def convert_security_group(security_group)
>> +          rules = []
>> +          security_group[:aws_perms].each do |perm|
>> +            sources = []
>> +            perm[:groups].each do |group|
>> +              sources<<  {:type =>  "group", :name =>  group[:group_name], :owner =>  group[:owner]}
>> +            end
>> +            perm[:ip_ranges].each do |ip|
>> +              sources<<  {:type =>  "address", :family=>"ipv4",
>> +                          :address=>ip[:cidr_ip].split("/").first,
>> +                          :prefix=>ip[:cidr_ip].split("/").last}
>> +            end
>> +            rule_id = firewall_rule_id(security_group[:aws_owner], perm[:protocol],
>> +                                       perm[:from_port] , perm[:to_port], sources)
>> +            rules<<  Firewall_Rule.new({:id =>  rule_id,
>
> Please use 'camel case' for class names in Ruby.
>
> <cite url="http://en.wikipedia.org/wiki/Naming_convention_(programming)">
> Ruby recommends UpperCamelCase for class names (e.g. UserProfile), lowercase words separated by underscores for variables (e.g. last_variable_used) and uppercase words separated by underscores for constants (e.g. WIKI_VERSION).
> </cite>
>
>> +                                        :allow_protocol =>  perm[:protocol],
>> +                                        :port_from =>  perm[:from_port],
>> +                                        :port_to =>  perm[:to_port],
>> +                                        :direction =>  'ingress',
>> +                                        :sources =>  sources})
>> +          end
>> +          Firewall.new(  {  :id =>  security_group[:aws_group_name],
>> +                            :name =>  security_group[:aws_group_name],
>> +                            :description =>  security_group[:aws_description],
>> +                            :owner_id =>  security_group[:aws_owner],
>> +                            :rules =>  rules
>> +                      }  )
>
> There is no need to use '{}' brackets in initializer, when there is only one Hash argument.
>
>> +        end
>> +
>>          def convert_state(ec2_state)
>>            case ec2_state
>>              when "terminated"
>> diff --git a/server/lib/deltacloud/models/firewall.rb b/server/lib/deltacloud/models/firewall.rb
>> new file mode 100644
>> index 0000000..dc0ae3d
>> --- /dev/null
>> +++ b/server/lib/deltacloud/models/firewall.rb
>> @@ -0,0 +1,22 @@
>> +#
>> +# Licensed to the Apache Software Foundation (ASF) under one or more
>> +# contributor license agreements.  See the NOTICE file distributed with
>> +# this work for additional information regarding copyright ownership.  The
>> +# ASF licenses this file to you under the Apache License, Version 2.0 (the
>> +# "License"); you may not use this file except in compliance with the
>> +# License.  You may obtain a copy of the License at
>> +#
>> +#       http://www.apache.org/licenses/LICENSE-2.0
>> +#
>> +# Unless required by applicable law or agreed to in writing, software
>> +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
>> +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
>> +# License for the specific language governing permissions and limitations
>> +# under the License.
>> +
>> +class Firewall<  BaseModel
>> +  attr_accessor :name
>> +  attr_accessor :description
>> +  attr_accessor :owner_id
>> +  attr_accessor :rules
>> +end
>> \ No newline at end of file
>> diff --git a/server/lib/deltacloud/models/firewall_rule.rb b/server/lib/deltacloud/models/firewall_rule.rb
>> new file mode 100644
>> index 0000000..df353a5
>> --- /dev/null
>> +++ b/server/lib/deltacloud/models/firewall_rule.rb
>> @@ -0,0 +1,23 @@
>> +#
>> +# Licensed to the Apache Software Foundation (ASF) under one or more
>> +# contributor license agreements.  See the NOTICE file distributed with
>> +# this work for additional information regarding copyright ownership.  The
>> +# ASF licenses this file to you under the Apache License, Version 2.0 (the
>> +# "License"); you may not use this file except in compliance with the
>> +# License.  You may obtain a copy of the License at
>> +#
>> +#       http://www.apache.org/licenses/LICENSE-2.0
>> +#
>> +# Unless required by applicable law or agreed to in writing, software
>> +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
>> +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
>> +# License for the specific language governing permissions and limitations
>> +# under the License.
>> +
>> +class Firewall_Rule<  BaseModel
>> +  attr_accessor :allow_protocol # tcp/udp/icmp
>> +  attr_accessor :port_from
>> +  attr_accessor :port_to
>> +  attr_accessor :sources
>> +  attr_accessor :direction #ingress egress
>> +end
>> diff --git a/server/server.rb b/server/server.rb
>> index 4da8357..a318df6 100644
>> --- a/server/server.rb
>> +++ b/server/server.rb
>> @@ -843,6 +843,7 @@ collection :buckets do
>>
>> end
>>
>> +
>> get '/api/addresses/:id/associate' do
>>    @instances = driver.instances(credentials)
>>    @address = Address::new(:id =>  params[:id])
>> @@ -929,3 +930,107 @@ collection :addresses do
>>    end
>>
>> end
>> +
>> +#html for creating a new firewall
>> +get '/api/firewalls/new' do
>> +  respond_to do |format|
>> +    format.html { haml :"firewalls/new" }
>> +  end
>> +end
>> +
>> +#html for creating a new firewall rule
>> +get '/api/firewalls/:firewall/new_rule' do
>> +  @firewall_name = params[:firewall]
>> +  respond_to do |format|
>> +    format.html {haml :"firewalls/new_rule" }
>> +  end
>> +end
>> +
>> +#create new firewall rule
>> +post '/api/firewalls/:firewall/rules' do
>> +  opts = {}
>> +  opts['firewall'] = params[:firewall] # must specify group_name or group_id
>> +  opts['protocol'] = params[:protocol]
>> +  opts['from_port'] = params[:from_port]
>> +  opts['to_port'] = params[:to_port]
>> +  addresses =  params.inject([]){|result,current| result<<  current.last unless current.grep(/^ip[-_]address/i).empty?; result}
>> +  groups = {}
>> +  max_groups  = params.select{|k,v| k=~/^group/}.size/2
>> +  for i in (1..max_groups) do
>> +    groups.merge!({params["group#{i}"]=>params["group#{i}owner"]})
>> +  end
>> +  opts['addresses'] = addresses
>> +  opts['groups'] = groups
>> +  driver.create_firewall_rule(credentials, opts)
>> +  @firewall = driver.firewall(credentials, {:id =>  params[:firewall]})
>
>> +  respond_to do |format|
>> +    format.html {haml :"firewalls/show"}
>> +    format.xml do
>> +      response.status = 201 #created
>> +      haml :"firewalls/show"
>> +    end
>> +  end
>> +end
>
> This should be moved to 'collection' block, defined like:
>
> operation :rules, :method =>  :post, :member =>  true
>    description "Blabla"
>    param :firewall, :required, :string
>    param :protocol, :required, :string
>    param :from_port, :required, :string
>    param :to_port, :required, :string
>    with_capability :create_firewall_rule
>    control do {
>      ...
>    }
> end
>
> Benefit of using this operation block inside collection is parameter
> validation, error handling and auto documentation. We should try to use
> Rabbit everywhere where it's possible.
>
>> +
>> +#delete a firewall rule
>> +delete '/api/firewalls/:firewall/:rule' do
>> +  opts = {}
>> +  opts[:firewall] = params[:firewall]
>> +  opts[:rule_id] = params[:rule]
>> +  driver.delete_firewall_rule(credentials, opts)
>> +  respond_to do |format|
>> +    format.html {redirect firewall_url(params[:firewall])}
>> +    format.xml {204}
>> +    format.json {204}
>> +  end
>> +end
>
> operation :rule, :method =>  :delete, :member =>  true
>    description "Blabla"
>    param :id,        :string, :required
>    param :rule_id,   :string, :required
>    with_capability :delete_firewall_rule
>    control do {
>      ...
>    }
> end
>
> ->  This will generate URI: DELETE /api/firewalls/123/rules
>
> I suggest to use parameter 'rule_id' to identify rule used for deletion.
>
>> +collection :firewalls do
>> +  description "Allow user to define firewall rules for an instance (ec2 security groups) eg expose ssh access [port 22, tcp]."
>> +
>> +  operation :index do
>> +    description 'List all firewalls'
>> +    with_capability :firewalls
>> +    control { filter_all(:firewalls) }
>> +  end
>> +
>> +  operation :show do
>> +    description 'Show details for a specific firewall - list all rules'
>> +    with_capability :firewall
>> +    param :id,            :string,    :required
>> +    control { show(:firewall) }
>> +  end
>> +
>> +  operation :create do
>> +    description 'Create a new firewall'
>> +    with_capability :create_firewall
>> +    param :name,          :string,    :required
>> +    param :description,   :string,    :required
>> +    control do
>> +      @firewall = driver.create_firewall(credentials, params )
>> +      respond_to do |format|
>> +        format.xml do
>> +          response.status = 201  # Created
>> +          haml :"firewalls/show"
>> +        end
>> +        format.html {haml :"firewalls/show"}
>> +      end
>> +    end
>> +  end
>> +
>> +  operation :destroy do
>> +    description 'Delete a specified firewall - error if firewall has rules'
>> +    with_capability :delete_firewall
>> +    param :id,            :string,    :required
>> +    control do
>> +      driver.delete_firewall(credentials, params)
>> +      respond_to do |format|
>> +        format.xml { 204 }
>> +        format.json {  204 }
>> +        format.html {  redirect(firewalls_url) }
>> +      end
>> +    end
>> +  end
>> +
>> +end #firewalls
>> diff --git a/server/views/firewalls/index.html.haml b/server/views/firewalls/index.html.haml
>> new file mode 100644
>> index 0000000..3312a32
>> --- /dev/null
>> +++ b/server/views/firewalls/index.html.haml
>> @@ -0,0 +1,25 @@
>> +%h1 Firewalls
>> +%br
>> +%p
>> +  =link_to 'Create new firewall', "/api/firewalls/new"
>> +%table.display
>> +  %thead
>> +    %tr
>> +      %th Id
>> +      %th Name
>> +      %th Description
>> +      %th Owner ID
>> +      %th Rules
>> +  %tbody
>> +    - @firewalls.each do |firewall|
>> +      %tr
>> +        %td
>> +          = link_to firewall.id, firewall_url(firewall.id)
>> +        %td
>> +          = firewall.name
>> +        %td
>> +          = firewall.description
>> +        %td
>> +          = firewall.owner_id
>> +        %td
>> +          = link_to 'view rules', firewall_url(firewall.id)
>> diff --git a/server/views/firewalls/index.xml.haml b/server/views/firewalls/index.xml.haml
>> new file mode 100644
>> index 0000000..8397f2d
>> --- /dev/null
>> +++ b/server/views/firewalls/index.xml.haml
>> @@ -0,0 +1,23 @@
>> +!!! XML
>> +%firewalls
>> +  - @firewalls.each do |firewall|
>> +    %firewall{:href =>  firewall_url(firewall.id), :id =>  firewall.id}
>> +      - firewall.attributes.select{ |attr| attr != :id&&  attr!= :rules}.each do |attribute|
>> +        - haml_tag("#{attribute}".tr('-', '_'), :<) do
>> +          - if [:name, :description].include?(attribute)
>> +            =cdata do
>> +              - haml_concat firewall.send(attribute)
>> +          - else
>> +            - haml_concat firewall.send(attribute)
>> +      %rules
>> +        - firewall.rules.each do |rule|
>> +          %rule
>> +            - rule.attributes.select{|attr| attr != :sources}.each do |rule_attrib|
>> +              - haml_tag("#{rule_attrib}".tr('-', '_'), :<) do
>> +                - haml_concat rule.send(rule_attrib)
>> +            %sources
>> +              - rule.sources.each do |source|
>> +                - if source[:type] == "group"
>> +                  %source{:name =>  source[:name], :type=>  source[:type], :owner=>  source[:owner]}
>> +                - else
>> +                  %source{:prefix =>  source[:prefix], :address=>  source[:address], :family=>source[:family], :type =>  source[:type]}
>> \ No newline at end of file
>> diff --git a/server/views/firewalls/new.html.haml b/server/views/firewalls/new.html.haml
>> new file mode 100644
>> index 0000000..4a230a6
>> --- /dev/null
>> +++ b/server/views/firewalls/new.html.haml
>> @@ -0,0 +1,11 @@
>> +%h1 New Firewall
>> +
>> +%form{:action =>  firewalls_url, :method =>  :post}
>> +  %label
>> +    Firewall Name
>> +    %input{:name =>  'name', :size =>  25}/
>> +    %br
>> +  %label
>> +    Firewall Description
>> +    %input{:name =>  'description', :size =>  100}/
>> +  %input{:type =>  :submit, :name =>  "commit", :value=>"create"}
>> \ No newline at end of file
>> diff --git a/server/views/firewalls/show.html.haml b/server/views/firewalls/show.html.haml
>> new file mode 100644
>> index 0000000..e6a186f
>> --- /dev/null
>> +++ b/server/views/firewalls/show.html.haml
>> @@ -0,0 +1,41 @@
>> +%h1 Firewall
>> +%h2
>> +  = @firewall.id
>> +%dl
>> +  %di
>> +    %dt Name
>> +    %dd
>> +      = @firewall.name
>> +    %dt Owner
>> +    %dd
>> +      = @firewall.owner_id
>> +    %dt Description
>> +    %dd
>> +      = @firewall.description
>> +
>> +%h2
>> +  Rules
>> +  %br
>> +  %p
>> +    =link_to 'Create a new rule', "/api/firewalls/#{@firewall.name}/new_rule"
>> +%dl
>> +  - @firewall.rules.each do |rule|
>> +    %di
>> +      Rule
>> +      - rule.attributes.select{|attr| attr != :sources}.each do |attrib|
>> +        %dt #{attrib}
>> +        %dd
>> +          = rule.send(attrib)
>> +      %dt sources
>> +      %dd
>> +        - rule.sources.each do |source|
>> +          - if source[:type] == "group"
>> +            type: #{source[:type]}, name: #{source[:name]}, owner: #{source[:owner]}
>> +            %br
>> +          - else
>> +            type: #{source[:type]}, family: #{source[:family]}, address: #{source[:address]}, prefix: #{source[:prefix]}
>> +            %br
>> +      %dd
>> +        = link_to_action 'Delete Rule', destroy_firewall_url(@firewall.name) + "/#{rule.id}", :delete
>> +  %dd
>> +    = link_to_action 'Delete Firewall', destroy_firewall_url(@firewall.name), :delete
>> \ No newline at end of file
>> diff --git a/server/views/firewalls/show.xml.haml b/server/views/firewalls/show.xml.haml
>> new file mode 100644
>> index 0000000..200d2cd
>> --- /dev/null
>> +++ b/server/views/firewalls/show.xml.haml
>> @@ -0,0 +1,21 @@
>> +!!! XML
>> +%firewall{:href =>  firewall_url(@firewall.id), :id =>  @firewall.id}
>> +  - @firewall.attributes.select{ |attr| attr != :id&&  attr!= :rules}.each do |attribute|
>> +    - haml_tag("#{attribute}".tr('-', '_'), :<) do
>> +      - if [:name, :description].include?(attribute)
>> +        =cdata do
>> +          - haml_concat @firewall.send(attribute)
>> +      - else
>> +        - haml_concat @firewall.send(attribute)
>> +  %rules
>> +    - @firewall.rules.each do |rule|
>> +      %rule
>> +        - rule.attributes.select{|attr| attr != :sources}.each do |rule_attrib|
>> +          - haml_tag("#{rule_attrib}".tr('-', '_'), :<) do
>> +            - haml_concat rule.send(rule_attrib)
>> +        %sources
>> +          - rule.sources.each do |source|
>> +            - if source[:type] == "group"
>> +              %source{:name =>  source[:name], :type=>  source[:type], :owner=>source[:owner]}
>> +            - else
>> +              %source{:prefix =>  source[:prefix], :address=>  source[:address], :family=>source[:family], :type =>  source[:type]}
>> \ No newline at end of file
>> --
>> 1.7.3.4
>>
>
> ------------------------------------------------------
> Michal Fojtik, mfojtik@redhat.com
> Deltacloud API: http://deltacloud.org
>


Re: [PATCH] implements 'firewalls' - which are ec2 security groups

Posted by Michal Fojtik <mf...@redhat.com>.
On May 16, 2011, at 9:06 AM, marios@redhat.com wrote:

Actually NACK.

However this patch works fine the only major doubt I have on it
are missing 'safely..end' block. This prevent reporting errors
to client correctly.

It would be also nice to have firewalls collection in Mock driver if it's
possible to run some tests on it, eventually add this stuff to EC2 Cucumber
features (I can help you with this).

  -- Michal

> From: marios <ma...@redhat.com>
> 
> 
> Signed-off-by: marios <ma...@redhat.com>
> ---
> server/deltacloud.rb                             |    2 +
> server/lib/deltacloud/base_driver/base_driver.rb |    6 +-
> server/lib/deltacloud/base_driver/features.rb    |    8 +-
> server/lib/deltacloud/drivers/ec2/ec2_driver.rb  |  142 +++++++++++++++++++++-
> server/lib/deltacloud/models/firewall.rb         |   22 ++++
> server/lib/deltacloud/models/firewall_rule.rb    |   23 ++++
> server/server.rb                                 |  105 ++++++++++++++++
> server/views/firewalls/index.html.haml           |   25 ++++
> server/views/firewalls/index.xml.haml            |   23 ++++
> server/views/firewalls/new.html.haml             |   11 ++
> server/views/firewalls/show.html.haml            |   41 ++++++
> server/views/firewalls/show.xml.haml             |   21 +++
> 12 files changed, 421 insertions(+), 8 deletions(-)
> create mode 100644 server/lib/deltacloud/models/firewall.rb
> create mode 100644 server/lib/deltacloud/models/firewall_rule.rb
> create mode 100644 server/views/firewalls/index.html.haml
> create mode 100644 server/views/firewalls/index.xml.haml
> create mode 100644 server/views/firewalls/new.html.haml
> create mode 100644 server/views/firewalls/show.html.haml
> create mode 100644 server/views/firewalls/show.xml.haml
> 
> diff --git a/server/deltacloud.rb b/server/deltacloud.rb
> index 7caf34f..5628e31 100644
> --- a/server/deltacloud.rb
> +++ b/server/deltacloud.rb
> @@ -36,6 +36,8 @@ require 'deltacloud/models/storage_volume'
> require 'deltacloud/models/bucket'
> require 'deltacloud/models/blob'
> require 'deltacloud/models/load_balancer'
> +require 'deltacloud/models/firewall'
> +require 'deltacloud/models/firewall_rule'
> 
> require 'deltacloud/validation'
> require 'deltacloud/helpers'
> diff --git a/server/lib/deltacloud/base_driver/base_driver.rb b/server/lib/deltacloud/base_driver/base_driver.rb
> index d9ebd92..06363d9 100644
> --- a/server/lib/deltacloud/base_driver/base_driver.rb
> +++ b/server/lib/deltacloud/base_driver/base_driver.rb
> @@ -183,8 +183,12 @@ module Deltacloud
>       keys(credentials, opts).first if has_capability?(:keys)
>     end
> 
> +    def firewall(credentials, opts={})
> +      firewalls(credentials, opts).first if has_capability?(:firewalls)
> +    end
> +
>     MEMBER_SHOW_METHODS =
> -      [ :realm, :image, :instance, :storage_volume, :bucket, :blob, :key ]
> +      [ :realm, :image, :instance, :storage_volume, :bucket, :blob, :key, :firewall ]
> 
>     def has_capability?(capability)
>       if MEMBER_SHOW_METHODS.include?(capability.to_sym)
> diff --git a/server/lib/deltacloud/base_driver/features.rb b/server/lib/deltacloud/base_driver/features.rb
> index 65c4cba..cb25a3b 100644
> --- a/server/lib/deltacloud/base_driver/features.rb
> +++ b/server/lib/deltacloud/base_driver/features.rb
> @@ -183,11 +183,11 @@ module Deltacloud
>       end
>     end
> 
> -    declare_feature :instances, :security_group do
> -      description "Put instance in one or more security groups on launch"
> +    declare_feature :instances, :firewall do
> +      description "Put instance in one or more firewalls (security groups) on launch"
>       operation :create do
> -        param :security_group, :array, :optional, nil,
> -        "Array of security group names"
> +        param :firewalls, :array, :optional, nil,
> +        "Array of firewall (security group) id"
>       end
>     end
> 
> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> index 14c5829..4ef6efd 100644
> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> @@ -16,6 +16,7 @@
> 
> require 'deltacloud/base_driver'
> require 'aws'
> +require 'base64'
> 
> class Instance
>   attr_accessor :keyname
> @@ -33,12 +34,13 @@ module Deltacloud
>       class EC2Driver < Deltacloud::BaseDriver
> 
>         def supported_collections
> -          DEFAULT_COLLECTIONS + [ :keys, :buckets, :load_balancers, :addresses ]
> +
> +          DEFAULT_COLLECTIONS + [ :keys, :buckets, :load_balancers, :addresses, :firewalls ]
>         end
> 
>         feature :instances, :user_data
>         feature :instances, :authentication_key
> -        feature :instances, :security_group
> +        feature :instances, :firewall
>         feature :instances, :instance_count
>         feature :images, :owner_id
>         feature :buckets, :bucket_location
> @@ -201,7 +203,7 @@ module Deltacloud
>           instance_options.merge!(:key_name => opts[:keyname]) if opts[:keyname]
>           instance_options.merge!(:availability_zone => opts[:realm_id]) if opts[:realm_id]
>           instance_options.merge!(:instance_type => opts[:hwp_id]) if opts[:hwp_id] && opts[:hwp_id].length > 0
> -          instance_options.merge!(:group_ids => opts[:security_group]) if opts[:security_group]
> +          instance_options.merge!(:group_ids => opts[:firewall]) if opts[:firewall]
>           instance_options.merge!(
>             :min_count => opts[:instance_count],
>             :max_count => opts[:instance_count]
> @@ -516,6 +518,7 @@ module Deltacloud
>           end
>         end
> 
> +
>         def addresses(credentials, opts={})
>           ec2 = new_client(credentials)
>           address_id = (opts and opts[:id]) ? [opts[:id]] : []
> @@ -571,6 +574,63 @@ module Deltacloud
>           end
>         end
> 
> +#--
> +#FIREWALLS - ec2 security groups
> +#--
> +			def firewalls(credentials, opts={})

^^ looks like a formatting typo. 

> +          ec2 = new_client(credentials)
> +          the_firewalls = []
> +          groups = []
> +          if opts[:id]
> +            groups = ec2.describe_security_groups([opts[:id]])
> +          else
> +            groups = ec2.describe_security_groups()
> +          end
> +          groups.each do |security_group|
> +            the_firewalls << 
> +          end

Could be replaced by:

the_firewalls = groups.collect { |security_group| convert_security_group(security_group) }

** Also 'safely..end' blocks are missing here and on other places bellow **.


> +          filter_on(the_firewalls, :id, opts)
> +		  end
> +
> +#--
> +#Create firewall
> +#--
> +        def create_firewall(credentials, opts={})
> +          ec2 = new_client(credentials)
> +          ec2.create_security_group(opts["name"], opts["description"])
> +          Firewall.new( { :id=>opts["name"], :name=>opts["name"],
> +                          :description => opts["description"], :owner_id => "", :rules => [] } )
> +        end
> +
> +#--
> +#Delete firewall
> +#--
> +        def delete_firewall(credentials, opts={})
> +          ec2 = new_client(credentials)
> +          ec2.delete_security_group(opts["id"])
> +        end
> +#--
> +#Create firewall rule
> +#--
> +        def create_firewall_rule(credentials, opts={})
> +          ec2 = new_client(credentials)
> +          groups = []
> +          opts['groups'].each do |k,v|
> +            groups << {"group_name" => k, "owner" =>v}
> +          end
> +          ec2.manage_security_group_ingress(opts['firewall'], opts['from_port'], opts['to_port'], opts['protocol'],
> +              "authorize", opts['addresses'], groups)
> +        end
> +#--
> +#Delete firewall rule
> +#--
> +        def delete_firewall_rule(credentials, opts={})
> +          ec2 = new_client(credentials)
> +          firewall = opts[:firewall]
> +          protocol, from_port, to_port, addresses, groups = firewall_rule_params(opts[:rule_id])
> +          ec2.manage_security_group_ingress(firewall, from_port, to_port, protocol, "revoke", addresses, groups)
> +        end
> +
>         def valid_credentials?(credentials)
>           retval = true
>           begin
> @@ -764,6 +824,82 @@ module Deltacloud
>           balancer
>         end
> 
> +        #generate uid from firewall rule parameters (amazon doesn't do this for us
> +        def firewall_rule_id(user_id, protocol, from_port, to_port, sources)
> +          sources_string = ""
> +          sources.each do |source|
> +            source.each_pair do |key,value|
> +              sources_string<< "#{key}=#{value}&"
> +            end
> +            sources_string.chomp!("&")
> +            sources_string<<"|"
> +          end
> +          sources_string.chomp!("|")
> +          #"type=group&owner=123456789012&name=new_firewall|type=address&family=ipv4&address=192.168.1.1&prefix=24"
> +          id_string = "user #{user_id}:::protocol #{protocol}:::from_port #{from_port}:::to_port #{to_port}:::sources #{sources_string}"
> +          Base64.encode64(id_string)
> +        end
> +
> +        #extract params from uid
> +        def firewall_rule_params(id)
> +          param_string = Base64.decode64(id)
> +          # "#{user_id}:::#{protocol}:::#{from_port}:::#{to_port}:::#{sources_string}"
> +          params = param_string.split(":::")
> +          protocol = params.grep(/protocol/).first.split(" ").last
> +          from_port = params.grep(/from_port/).first.split(" ").last
> +          to_port = params.grep(/to_port/).first.split(" ").last
> +          sources = params.grep(/sources/).first.split(" ").last
> +          addresses = []
> +          groups = []
> +          sources.split("|").each do |source|
> +            current = source.split("&")
> +            type = current.grep(/type/).first.split("=").last
> +            case type
> +              when 'group'
> +                #type=group&owner=123456789012&name=default
> +                name = current.grep(/name/).first.split("=").last
> +                owner = current.grep(/owner/).first.split("=").last
> +                groups << {'group_name' => name, 'owner' => owner}
> +              when 'address'
> +                #type=address&family=ipv4&address=10.1.1.1&prefix=24
> +                address = current.grep(/address/).last.split("=").last
> +                address<<"/#{current.grep(/prefix/).first.split("=").last}"
> +                addresses << address
> +            end
> +          end
> +          return protocol, from_port, to_port, addresses, groups
> +        end
> +
> +        #Convert ec2 security group to server/lib/deltacloud/models/firewall
> +        def convert_security_group(security_group)
> +          rules = []
> +          security_group[:aws_perms].each do |perm|
> +            sources = []
> +            perm[:groups].each do |group|
> +              sources << {:type => "group", :name => group[:group_name], :owner => group[:owner]}
> +            end
> +            perm[:ip_ranges].each do |ip|
> +              sources << {:type => "address", :family=>"ipv4",
> +                          :address=>ip[:cidr_ip].split("/").first,
> +                          :prefix=>ip[:cidr_ip].split("/").last}
> +            end
> +            rule_id = firewall_rule_id(security_group[:aws_owner], perm[:protocol],
> +                                       perm[:from_port] , perm[:to_port], sources)
> +            rules << Firewall_Rule.new({:id => rule_id,

Please use 'camel case' for class names in Ruby.

<cite url="http://en.wikipedia.org/wiki/Naming_convention_(programming)">
Ruby recommends UpperCamelCase for class names (e.g. UserProfile), lowercase words separated by underscores for variables (e.g. last_variable_used) and uppercase words separated by underscores for constants (e.g. WIKI_VERSION).
</cite>

> +                                        :allow_protocol => perm[:protocol],
> +                                        :port_from => perm[:from_port],
> +                                        :port_to => perm[:to_port],
> +                                        :direction => 'ingress',
> +                                        :sources => sources})
> +          end
> +          Firewall.new(  {  :id => security_group[:aws_group_name],
> +                            :name => security_group[:aws_group_name],
> +                            :description => security_group[:aws_description],
> +                            :owner_id => security_group[:aws_owner],
> +                            :rules => rules
> +                      }  )

There is no need to use '{}' brackets in initializer, when there is only one Hash argument.

> +        end
> +
>         def convert_state(ec2_state)
>           case ec2_state
>             when "terminated"
> diff --git a/server/lib/deltacloud/models/firewall.rb b/server/lib/deltacloud/models/firewall.rb
> new file mode 100644
> index 0000000..dc0ae3d
> --- /dev/null
> +++ b/server/lib/deltacloud/models/firewall.rb
> @@ -0,0 +1,22 @@
> +#
> +# Licensed to the Apache Software Foundation (ASF) under one or more
> +# contributor license agreements.  See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.  The
> +# ASF licenses this file to you under the Apache License, Version 2.0 (the
> +# "License"); you may not use this file except in compliance with the
> +# License.  You may obtain a copy of the License at
> +#
> +#       http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
> +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
> +# License for the specific language governing permissions and limitations
> +# under the License.
> +
> +class Firewall < BaseModel
> +  attr_accessor :name
> +  attr_accessor :description
> +  attr_accessor :owner_id
> +  attr_accessor :rules
> +end
> \ No newline at end of file
> diff --git a/server/lib/deltacloud/models/firewall_rule.rb b/server/lib/deltacloud/models/firewall_rule.rb
> new file mode 100644
> index 0000000..df353a5
> --- /dev/null
> +++ b/server/lib/deltacloud/models/firewall_rule.rb
> @@ -0,0 +1,23 @@
> +#
> +# Licensed to the Apache Software Foundation (ASF) under one or more
> +# contributor license agreements.  See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.  The
> +# ASF licenses this file to you under the Apache License, Version 2.0 (the
> +# "License"); you may not use this file except in compliance with the
> +# License.  You may obtain a copy of the License at
> +#
> +#       http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
> +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
> +# License for the specific language governing permissions and limitations
> +# under the License.
> +
> +class Firewall_Rule < BaseModel
> +  attr_accessor :allow_protocol # tcp/udp/icmp
> +  attr_accessor :port_from
> +  attr_accessor :port_to
> +  attr_accessor :sources
> +  attr_accessor :direction #ingress egress
> +end
> diff --git a/server/server.rb b/server/server.rb
> index 4da8357..a318df6 100644
> --- a/server/server.rb
> +++ b/server/server.rb
> @@ -843,6 +843,7 @@ collection :buckets do
> 
> end
> 
> +
> get '/api/addresses/:id/associate' do
>   @instances = driver.instances(credentials)
>   @address = Address::new(:id => params[:id])
> @@ -929,3 +930,107 @@ collection :addresses do
>   end
> 
> end
> +
> +#html for creating a new firewall
> +get '/api/firewalls/new' do
> +  respond_to do |format|
> +    format.html { haml :"firewalls/new" }
> +  end
> +end
> +
> +#html for creating a new firewall rule
> +get '/api/firewalls/:firewall/new_rule' do
> +  @firewall_name = params[:firewall]
> +  respond_to do |format|
> +    format.html {haml :"firewalls/new_rule" }
> +  end
> +end
> +
> +#create new firewall rule
> +post '/api/firewalls/:firewall/rules' do
> +  opts = {}
> +  opts['firewall'] = params[:firewall] # must specify group_name or group_id
> +  opts['protocol'] = params[:protocol]
> +  opts['from_port'] = params[:from_port]
> +  opts['to_port'] = params[:to_port]
> +  addresses =  params.inject([]){|result,current| result << current.last unless current.grep(/^ip[-_]address/i).empty?; result}
> +  groups = {}
> +  max_groups  = params.select{|k,v| k=~/^group/}.size/2
> +  for i in (1..max_groups) do
> +    groups.merge!({params["group#{i}"]=>params["group#{i}owner"]})
> +  end
> +  opts['addresses'] = addresses
> +  opts['groups'] = groups
> +  driver.create_firewall_rule(credentials, opts)
> +  @firewall = driver.firewall(credentials, {:id => params[:firewall]})

> +  respond_to do |format|
> +    format.html {haml :"firewalls/show"}
> +    format.xml do
> +      response.status = 201 #created
> +      haml :"firewalls/show"
> +    end
> +  end
> +end

This should be moved to 'collection' block, defined like:

operation :rules, :method => :post, :member => true
  description "Blabla"
  param :firewall, :required, :string
  param :protocol, :required, :string
  param :from_port, :required, :string
  param :to_port, :required, :string
  with_capability :create_firewall_rule
  control do {
    ...
  }
end

Benefit of using this operation block inside collection is parameter
validation, error handling and auto documentation. We should try to use
Rabbit everywhere where it's possible.

> +
> +#delete a firewall rule
> +delete '/api/firewalls/:firewall/:rule' do
> +  opts = {}
> +  opts[:firewall] = params[:firewall]
> +  opts[:rule_id] = params[:rule]
> +  driver.delete_firewall_rule(credentials, opts)
> +  respond_to do |format|
> +    format.html {redirect firewall_url(params[:firewall])}
> +    format.xml {204}
> +    format.json {204}
> +  end
> +end

operation :rule, :method => :delete, :member => true
  description "Blabla"
  param :id,        :string, :required
  param :rule_id,   :string, :required
  with_capability :delete_firewall_rule
  control do {
    ...
  }
end

-> This will generate URI: DELETE /api/firewalls/123/rules

I suggest to use parameter 'rule_id' to identify rule used for deletion.

> +collection :firewalls do
> +  description "Allow user to define firewall rules for an instance (ec2 security groups) eg expose ssh access [port 22, tcp]."
> +
> +  operation :index do
> +    description 'List all firewalls'
> +    with_capability :firewalls
> +    control { filter_all(:firewalls) }
> +  end
> +
> +  operation :show do
> +    description 'Show details for a specific firewall - list all rules'
> +    with_capability :firewall
> +    param :id,            :string,    :required
> +    control { show(:firewall) }
> +  end
> +
> +  operation :create do
> +    description 'Create a new firewall'
> +    with_capability :create_firewall
> +    param :name,          :string,    :required
> +    param :description,   :string,    :required
> +    control do
> +      @firewall = driver.create_firewall(credentials, params )
> +      respond_to do |format|
> +        format.xml do
> +          response.status = 201  # Created
> +          haml :"firewalls/show"
> +        end
> +        format.html {haml :"firewalls/show"}
> +      end
> +    end
> +  end
> +
> +  operation :destroy do
> +    description 'Delete a specified firewall - error if firewall has rules'
> +    with_capability :delete_firewall
> +    param :id,            :string,    :required
> +    control do
> +      driver.delete_firewall(credentials, params)
> +      respond_to do |format|
> +        format.xml { 204 }
> +        format.json {  204 }
> +        format.html {  redirect(firewalls_url) }
> +      end
> +    end
> +  end
> +
> +end #firewalls
> diff --git a/server/views/firewalls/index.html.haml b/server/views/firewalls/index.html.haml
> new file mode 100644
> index 0000000..3312a32
> --- /dev/null
> +++ b/server/views/firewalls/index.html.haml
> @@ -0,0 +1,25 @@
> +%h1 Firewalls
> +%br
> +%p
> +  =link_to 'Create new firewall', "/api/firewalls/new"
> +%table.display
> +  %thead
> +    %tr
> +      %th Id
> +      %th Name
> +      %th Description
> +      %th Owner ID
> +      %th Rules
> +  %tbody
> +    - @firewalls.each do |firewall|
> +      %tr
> +        %td
> +          = link_to firewall.id, firewall_url(firewall.id)
> +        %td
> +          = firewall.name
> +        %td
> +          = firewall.description
> +        %td
> +          = firewall.owner_id
> +        %td
> +          = link_to 'view rules', firewall_url(firewall.id)
> diff --git a/server/views/firewalls/index.xml.haml b/server/views/firewalls/index.xml.haml
> new file mode 100644
> index 0000000..8397f2d
> --- /dev/null
> +++ b/server/views/firewalls/index.xml.haml
> @@ -0,0 +1,23 @@
> +!!! XML
> +%firewalls
> +  - @firewalls.each do |firewall|
> +    %firewall{:href => firewall_url(firewall.id), :id => firewall.id}
> +      - firewall.attributes.select{ |attr| attr != :id && attr!= :rules}.each do |attribute|
> +        - haml_tag("#{attribute}".tr('-', '_'), :<) do
> +          - if [:name, :description].include?(attribute)
> +            =cdata do
> +              - haml_concat firewall.send(attribute)
> +          - else
> +            - haml_concat firewall.send(attribute)
> +      %rules
> +        - firewall.rules.each do |rule|
> +          %rule
> +            - rule.attributes.select{|attr| attr != :sources}.each do |rule_attrib|
> +              - haml_tag("#{rule_attrib}".tr('-', '_'), :<) do
> +                - haml_concat rule.send(rule_attrib)
> +            %sources
> +              - rule.sources.each do |source|
> +                - if source[:type] == "group"
> +                  %source{:name => source[:name], :type=> source[:type], :owner=> source[:owner]}
> +                - else
> +                  %source{:prefix => source[:prefix], :address=> source[:address], :family=>source[:family], :type => source[:type]}
> \ No newline at end of file
> diff --git a/server/views/firewalls/new.html.haml b/server/views/firewalls/new.html.haml
> new file mode 100644
> index 0000000..4a230a6
> --- /dev/null
> +++ b/server/views/firewalls/new.html.haml
> @@ -0,0 +1,11 @@
> +%h1 New Firewall
> +
> +%form{:action => firewalls_url, :method => :post}
> +  %label
> +    Firewall Name
> +    %input{:name => 'name', :size => 25}/
> +    %br
> +  %label
> +    Firewall Description
> +    %input{:name => 'description', :size => 100}/
> +  %input{:type => :submit, :name => "commit", :value=>"create"}
> \ No newline at end of file
> diff --git a/server/views/firewalls/show.html.haml b/server/views/firewalls/show.html.haml
> new file mode 100644
> index 0000000..e6a186f
> --- /dev/null
> +++ b/server/views/firewalls/show.html.haml
> @@ -0,0 +1,41 @@
> +%h1 Firewall
> +%h2
> +  = @firewall.id
> +%dl
> +  %di
> +    %dt Name
> +    %dd
> +      = @firewall.name
> +    %dt Owner
> +    %dd
> +      = @firewall.owner_id
> +    %dt Description
> +    %dd
> +      = @firewall.description
> +
> +%h2
> +  Rules
> +  %br
> +  %p
> +    =link_to 'Create a new rule', "/api/firewalls/#{@firewall.name}/new_rule"
> +%dl
> +  - @firewall.rules.each do |rule|
> +    %di
> +      Rule
> +      - rule.attributes.select{|attr| attr != :sources}.each do |attrib|
> +        %dt #{attrib}
> +        %dd
> +          = rule.send(attrib)
> +      %dt sources
> +      %dd
> +        - rule.sources.each do |source|
> +          - if source[:type] == "group"
> +            type: #{source[:type]}, name: #{source[:name]}, owner: #{source[:owner]}
> +            %br
> +          - else
> +            type: #{source[:type]}, family: #{source[:family]}, address: #{source[:address]}, prefix: #{source[:prefix]}
> +            %br
> +      %dd
> +        = link_to_action 'Delete Rule', destroy_firewall_url(@firewall.name) + "/#{rule.id}", :delete
> +  %dd
> +    = link_to_action 'Delete Firewall', destroy_firewall_url(@firewall.name), :delete
> \ No newline at end of file
> diff --git a/server/views/firewalls/show.xml.haml b/server/views/firewalls/show.xml.haml
> new file mode 100644
> index 0000000..200d2cd
> --- /dev/null
> +++ b/server/views/firewalls/show.xml.haml
> @@ -0,0 +1,21 @@
> +!!! XML
> +%firewall{:href => firewall_url(@firewall.id), :id => @firewall.id}
> +  - @firewall.attributes.select{ |attr| attr != :id && attr!= :rules}.each do |attribute|
> +    - haml_tag("#{attribute}".tr('-', '_'), :<) do
> +      - if [:name, :description].include?(attribute)
> +        =cdata do
> +          - haml_concat @firewall.send(attribute)
> +      - else
> +        - haml_concat @firewall.send(attribute)
> +  %rules
> +    - @firewall.rules.each do |rule|
> +      %rule
> +        - rule.attributes.select{|attr| attr != :sources}.each do |rule_attrib|
> +          - haml_tag("#{rule_attrib}".tr('-', '_'), :<) do
> +            - haml_concat rule.send(rule_attrib)
> +        %sources
> +          - rule.sources.each do |source|
> +            - if source[:type] == "group"
> +              %source{:name => source[:name], :type=> source[:type], :owner=>source[:owner]}
> +            - else
> +              %source{:prefix => source[:prefix], :address=> source[:address], :family=>source[:family], :type => source[:type]}
> \ No newline at end of file
> -- 
> 1.7.3.4
> 

------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org