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.

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

Standardize Headers #796

Merged
merged 6 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Guardfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ guard 'rspec', rspec_options do
watch('lib/flipper/api/middleware.rb') { 'spec/flipper/api_spec.rb' }
watch(/shared_adapter_specs\.rb$/) { 'spec' }
watch('spec/helper.rb') { 'spec' }
watch('lib/flipper/adapters/http/client.rb') { 'spec/flipper/adapters/http_spec.rb' }

# To run all specs on every change... (useful with focus and fit)
# watch(%r{.*}) { 'spec' }
Expand Down
41 changes: 25 additions & 16 deletions lib/flipper/adapters/http/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,45 @@ module Adapters
class Http
class Client
DEFAULT_HEADERS = {
'Content-Type' => 'application/json',
'Accept' => 'application/json',
'User-Agent' => "Flipper HTTP Adapter v#{VERSION}",
'content-type' => 'application/json',
'accept' => 'application/json',
'user-agent' => "Flipper HTTP Adapter v#{VERSION}",
}.freeze

HTTPS_SCHEME = "https".freeze

CLIENT_FRAMEWORKS = {
rails: -> { Rails.version if defined?(Rails) },
sinatra: -> { Sinatra::VERSION if defined?(Sinatra) },
hanami: -> { Hanami::VERSION if defined?(Hanami) },
rails: -> { Rails.version if defined?(Rails) },
sinatra: -> { Sinatra::VERSION if defined?(Sinatra) },
hanami: -> { Hanami::VERSION if defined?(Hanami) },
sidekiq: -> { Sidekiq::VERSION if defined?(Sidekiq) },
good_job: -> { GoodJob::VERSION if defined?(GoodJob) },
}

attr_reader :uri, :headers
attr_reader :basic_auth_username, :basic_auth_password
attr_reader :read_timeout, :open_timeout, :write_timeout, :max_retries, :debug_output
attr_reader :read_timeout, :open_timeout, :write_timeout
attr_reader :max_retries, :debug_output

def initialize(options = {})
@uri = URI(options.fetch(:url))
@headers = DEFAULT_HEADERS.merge(options[:headers] || {})
@basic_auth_username = options[:basic_auth_username]
@basic_auth_password = options[:basic_auth_password]
@read_timeout = options[:read_timeout]
@open_timeout = options[:open_timeout]
@write_timeout = options[:write_timeout]
@max_retries = options.key?(:max_retries) ? options[:max_retries] : 0
@debug_output = options[:debug_output]

@headers = {}
DEFAULT_HEADERS.each { |key, value| add_header key, value }
if options[:headers]
options[:headers].each { |key, value| add_header key, value }
end
end

def add_header(key, value)
key = key.to_s.downcase.gsub('_'.freeze, '-'.freeze).freeze
@headers[key] = value
end

Expand Down Expand Up @@ -87,21 +96,21 @@ def build_http(uri)

def build_request(http_method, uri, headers, options)
request_headers = {
client_language: "ruby",
client_language_version: "#{RUBY_VERSION} p#{RUBY_PATCHLEVEL} (#{RUBY_RELEASE_DATE})",
client_platform: RUBY_PLATFORM,
client_engine: defined?(RUBY_ENGINE) ? RUBY_ENGINE : "",
client_pid: Process.pid.to_s,
client_thread: Thread.current.object_id.to_s,
client_hostname: Socket.gethostname,
'client-language' => "ruby",
'client-language-version' => "#{RUBY_VERSION} p#{RUBY_PATCHLEVEL} (#{RUBY_RELEASE_DATE})",
'client-platform' => RUBY_PLATFORM,
'client-engine' => defined?(RUBY_ENGINE) ? RUBY_ENGINE : "",
'client-pid' => Process.pid.to_s,
'client-thread' => Thread.current.object_id.to_s,
'client-hostname' => Socket.gethostname,
}.merge(headers)

body = options[:body]
request = http_method.new(uri.request_uri)
request.initialize_http_header(request_headers)

client_frameworks.each do |framework, version|
request.add_field("Client-Framework", [framework, version].join("="))
request.add_field("client-framework", [framework, version].join("="))
end

request.body = body if body
Expand Down
9 changes: 7 additions & 2 deletions lib/flipper/cloud/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ def http_adapter
max_retries: 0, # we'll handle retries ourselves
debug_output: @debug_output,
headers: {
flipper_cloud_token: @token,
accept_encoding: "gzip",
"flipper-cloud-token" => @token,
"accept-encoding" => "gzip",
},
})
end
Expand All @@ -187,6 +187,11 @@ def setup_log(options)
def setup_http(options)
set_option :url, options, default: DEFAULT_URL
set_option :debug_output, options, from_env: false

if @debug_output.nil? && Flipper::Typecast.to_boolean(ENV["FLIPPER_CLOUD_DEBUG_OUTPUT_STDOUT"])
@debug_output = STDOUT
end

set_option :read_timeout, options, default: 5, typecast: :float, minimum: 0.1
set_option :open_timeout, options, default: 2, typecast: :float, minimum: 0.1
set_option :write_timeout, options, default: 5, typecast: :float, minimum: 0.1
Expand Down
8 changes: 4 additions & 4 deletions lib/flipper/cloud/middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ def call!(env)
})
rescue Flipper::Adapters::Http::Error => error
status = error.response.code.to_i == 402 ? 402 : 500
headers["Flipper-Cloud-Response-Error-Class"] = error.class.name
headers["Flipper-Cloud-Response-Error-Message"] = error.message
headers["flipper-cloud-response-error-class"] = error.class.name
headers["flipper-cloud-response-error-message"] = error.message
rescue => error
status = 500
headers["Flipper-Cloud-Response-Error-Class"] = error.class.name
headers["Flipper-Cloud-Response-Error-Message"] = error.message
headers["flipper-cloud-response-error-class"] = error.class.name
headers["flipper-cloud-response-error-message"] = error.message
end
end
rescue MessageVerifier::InvalidSignature
Expand Down
4 changes: 2 additions & 2 deletions lib/flipper/cloud/telemetry/submitter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ def retry_with_backoff(attempts, &block)

def submit(body)
client = @cloud_configuration.http_client
client.add_header :schema_version, SCHEMA_VERSION
client.add_header :content_encoding, GZIP_ENCODING
client.add_header "schema-version", SCHEMA_VERSION
client.add_header "content-encoding", GZIP_ENCODING

response = client.post PATH, body
code = response.code.to_i
Expand Down
61 changes: 61 additions & 0 deletions spec/flipper/adapters/http/client_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
require "flipper/adapters/http/client"

RSpec.describe Flipper::Adapters::Http::Client do
describe "#initialize" do
it "requires url" do
expect { described_class.new }.to raise_error(KeyError, "key not found: :url")
end

it "sets default headers" do
client = described_class.new(url: "http://example.com")
expect(client.headers).to eq({
'content-type' => 'application/json',
'accept' => 'application/json',
'user-agent' => "Flipper HTTP Adapter v#{Flipper::VERSION}",
})
end

it "adds custom headers" do
client = described_class.new(url: "http://example.com", headers: {'custom-header' => 'value'})
expect(client.headers).to include('custom-header' => 'value')
end

it "overrides default headers with custom headers" do
client = described_class.new(url: "http://example.com", headers: {'content-type' => 'text/plain'})
expect(client.headers['content-type']).to eq('text/plain')
end
end

describe "#add_header" do
it "can add string header" do
client = described_class.new(url: "http://example.com")
client.add_header("key", "value")
expect(client.headers.fetch("key")).to eq("value")
end

it "standardizes key to lowercase" do
client = described_class.new(url: "http://example.com")
client.add_header("Content-Type", "value")
expect(client.headers.fetch("content-type")).to eq("value")
end

it "standardizes key to dashes" do
client = described_class.new(url: "http://example.com")
client.add_header(:content_type, "value")
expect(client.headers.fetch("content-type")).to eq("value")
end

it "can add symbol header" do
client = described_class.new(url: "http://example.com")
client.add_header(:key, "value")
expect(client.headers.fetch("key")).to eq("value")
end

it "overrides existing header" do
client = described_class.new(url: "http://example.com")
client.add_header("key", "value 1")
client.add_header("key", "value 2")
expect(client.headers.fetch("key")).to eq("value 2")
end
end
end
24 changes: 16 additions & 8 deletions spec/flipper/adapters/http_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

{
basic: default_options.dup,
gzip: default_options.dup.merge(headers: { accept_encoding: 'gzip' }),
gzip: default_options.dup.merge(headers: { 'accept-encoding': 'gzip' }),
}.each do |name, options|
context "adapter (#{name} #{options.inspect})" do
subject do
Expand Down Expand Up @@ -87,9 +87,9 @@

it "sends default headers" do
headers = {
'Accept' => 'application/json',
'Content-Type' => 'application/json',
'User-Agent' => "Flipper HTTP Adapter v#{Flipper::VERSION}",
'accept' => 'application/json',
'content-type' => 'application/json',
'user-agent' => "Flipper HTTP Adapter v#{Flipper::VERSION}",
}
stub_request(:get, "http://app.com/flipper/features/feature_panel")
.with(headers: headers)
Expand All @@ -103,9 +103,17 @@
stub_const("Rails", double(version: "7.1.0"))
stub_const("Sinatra::VERSION", "3.1.0")
stub_const("Hanami::VERSION", "0.7.2")
stub_const("GoodJob::VERSION", "3.21.5")
stub_const("Sidekiq::VERSION", "7.2.0")

headers = {
"Client-Framework" => ["rails=7.1.0", "sinatra=3.1.0", "hanami=0.7.2"]
"client-framework" => [
"rails=7.1.0",
"sinatra=3.1.0",
"hanami=0.7.2",
"good_job=3.21.5",
"sidekiq=7.2.0",
]
}

stub_request(:get, "http://app.com/flipper/features/feature_panel")
Expand All @@ -121,7 +129,7 @@
stub_const("Sinatra::VERSION", "3.1.0")

headers = {
"Client-Framework" => ["rails=7.1.0", "sinatra=3.1.0"]
"client-framework" => ["rails=7.1.0", "sinatra=3.1.0"]
}

stub_request(:get, "http://app.com/flipper/features/feature_panel")
Expand Down Expand Up @@ -289,7 +297,7 @@
let(:options) do
{
url: 'http://app.com/mount-point',
headers: { 'X-Custom-Header' => 'foo' },
headers: { 'x-custom-header' => 'foo' },
basic_auth_username: 'username',
basic_auth_password: 'password',
read_timeout: 100,
Expand All @@ -310,7 +318,7 @@
subject.get(feature)
expect(
a_request(:get, 'http://app.com/mount-point/features/feature_panel')
.with(headers: { 'X-Custom-Header' => 'foo' })
.with(headers: { 'x-custom-header' => 'foo' })
).to have_been_made.once
end

Expand Down
11 changes: 9 additions & 2 deletions spec/flipper/cloud/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@
expect(instance.debug_output).to eq(STDOUT)
end

it "defaults debug_output to STDOUT if FLIPPER_CLOUD_DEBUG_OUTPUT_STDOUT set to true" do
with_env "FLIPPER_CLOUD_DEBUG_OUTPUT_STDOUT" => "true" do
instance = described_class.new(required_options)
expect(instance.debug_output).to eq(STDOUT)
end
end

it "defaults adapter block" do
# The initial sync of http to local invokes this web request.
stub_request(:get, /flippercloud\.io/).to_return(status: 200, body: "{}")
Expand Down Expand Up @@ -233,9 +240,9 @@
stub = stub_request(:get, "https://www.flippercloud.io/adapter/features?exclude_gate_names=true").
with({
headers: {
'Flipper-Cloud-Token'=>'asdf',
'flipper-cloud-token'=>'asdf',
},
}).to_return(status: 200, body: body, headers: {})
}).to_return(status: 200, body: body)
instance = described_class.new(required_options)
instance.sync

Expand Down
10 changes: 5 additions & 5 deletions spec/flipper/cloud/dsl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
stub = stub_request(:get, "https://www.flippercloud.io/adapter/features?exclude_gate_names=true").
with({
headers: {
'Flipper-Cloud-Token'=>'asdf',
'flipper-cloud-token'=>'asdf',
},
}).to_return(status: 200, body: '{"features": {}}', headers: {})
cloud_configuration = Flipper::Cloud::Configuration.new({
Expand Down Expand Up @@ -65,11 +65,11 @@

it "sends writes to cloud and local" do
add_stub = stub_request(:post, "https://www.flippercloud.io/adapter/features").
with({headers: {'Flipper-Cloud-Token'=>'asdf'}}).
to_return(status: 200, body: '{}', headers: {})
with({headers: {'flipper-cloud-token'=>'asdf'}}).
to_return(status: 200, body: '{}')
enable_stub = stub_request(:post, "https://www.flippercloud.io/adapter/features/foo/boolean").
with(headers: {'Flipper-Cloud-Token'=>'asdf'}).
to_return(status: 200, body: '{}', headers: {})
with(headers: {'flipper-cloud-token'=>'asdf'}).
to_return(status: 200, body: '{}')

subject.enable(:foo)

Expand Down
16 changes: 8 additions & 8 deletions spec/flipper/cloud/middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@
post '/', request_body, env

expect(last_response.status).to eq(402)
expect(last_response.headers["Flipper-Cloud-Response-Error-Class"]).to eq("Flipper::Adapters::Http::Error")
expect(last_response.headers["Flipper-Cloud-Response-Error-Message"]).to include("Failed with status: 402")
expect(last_response.headers["flipper-cloud-response-error-class"]).to eq("Flipper::Adapters::Http::Error")
expect(last_response.headers["flipper-cloud-response-error-message"]).to include("Failed with status: 402")
expect(stub).to have_been_requested
end
end
Expand All @@ -124,8 +124,8 @@
post '/', request_body, env

expect(last_response.status).to eq(500)
expect(last_response.headers["Flipper-Cloud-Response-Error-Class"]).to eq("Flipper::Adapters::Http::Error")
expect(last_response.headers["Flipper-Cloud-Response-Error-Message"]).to include("Failed with status: 503")
expect(last_response.headers["flipper-cloud-response-error-class"]).to eq("Flipper::Adapters::Http::Error")
expect(last_response.headers["flipper-cloud-response-error-message"]).to include("Failed with status: 503")
expect(stub).to have_been_requested
end
end
Expand All @@ -147,8 +147,8 @@
post '/', request_body, env

expect(last_response.status).to eq(500)
expect(last_response.headers["Flipper-Cloud-Response-Error-Class"]).to eq("Net::OpenTimeout")
expect(last_response.headers["Flipper-Cloud-Response-Error-Message"]).to eq("execution expired")
expect(last_response.headers["flipper-cloud-response-error-class"]).to eq("Net::OpenTimeout")
expect(last_response.headers["flipper-cloud-response-error-message"]).to eq("execution expired")
expect(stub).to have_been_requested
end
end
Expand Down Expand Up @@ -277,13 +277,13 @@ def stub_request_for_token(token, status: 200)
stub = stub_request(:get, "https://www.flippercloud.io/adapter/features?exclude_gate_names=true").
with({
headers: {
'Flipper-Cloud-Token' => token,
'flipper-cloud-token' => token,
},
})
if status == :timeout
stub.to_timeout
else
stub.to_return(status: status, body: response_body, headers: {})
stub.to_return(status: status, body: response_body)
end
end
end
Loading