From 043aca2df43f1a9207efac909b6eb1c4afd83be3 Mon Sep 17 00:00:00 2001 From: Kevin Date: Fri, 29 Sep 2023 19:10:13 -0700 Subject: [PATCH 1/2] Revert "Revert "service: support multiple sockets in DSL"" --- Library/Homebrew/service.rb | 57 ++++++++---- Library/Homebrew/test/service_spec.rb | 123 +++++++++++++++++++------- docs/Formula-Cookbook.md | 18 ++++ 3 files changed, 149 insertions(+), 49 deletions(-) diff --git a/Library/Homebrew/service.rb b/Library/Homebrew/service.rb index 130e2f1e7e..3d1de39211 100644 --- a/Library/Homebrew/service.rb +++ b/Library/Homebrew/service.rb @@ -187,17 +187,26 @@ module Homebrew end end - sig { params(value: T.nilable(String)).returns(T.nilable(T::Hash[Symbol, String])) } + SOCKET_STRING_REGEX = %r{([a-z]+)://([a-z0-9.]+):([0-9]+)}i.freeze + + sig { + params(value: T.nilable(T.any(String, T::Hash[Symbol, String]))) + .returns(T.nilable(T::Hash[Symbol, T::Hash[Symbol, String]])) + } def sockets(value = nil) - case value - when nil - @sockets + return @sockets if value.nil? + + @sockets = case value when String - match = T.must(value).match(%r{([a-z]+)://([a-z0-9.]+):([0-9]+)}i) + { listeners: value } + when Hash + value + end.transform_values do |socket_string| + match = socket_string.match(SOCKET_STRING_REGEX) raise TypeError, "Service#sockets a formatted socket definition as ://:" if match.blank? type, host, port = match.captures - @sockets = { host: host, port: port, type: type } + { host: host, port: port, type: type } end end @@ -410,12 +419,14 @@ module Homebrew if @sockets.present? base[:Sockets] = {} - base[:Sockets][:Listeners] = { - SockNodeName: @sockets[:host], - SockServiceName: @sockets[:port], - SockProtocol: @sockets[:type].upcase, - SockFamily: "IPv4v6", - } + @sockets.each do |name, info| + base[:Sockets][name] = { + SockNodeName: info[:host], + SockServiceName: info[:port], + SockProtocol: info[:type].upcase, + SockFamily: "IPv4v6", + } + end end if @cron.present? && @run_type == RUN_TYPE_CRON @@ -511,7 +522,11 @@ module Homebrew .join(" ") end - sockets_string = "#{@sockets[:type]}://#{@sockets[:host]}:#{@sockets[:port]}" if @sockets.present? + sockets_hash = if @sockets.present? + @sockets.transform_values do |info| + "#{info[:type]}://#{info[:host]}:#{info[:port]}" + end + end { name: name_params.presence, @@ -531,7 +546,7 @@ module Homebrew restart_delay: @restart_delay, process_type: @process_type, macos_legacy_timers: @macos_legacy_timers, - sockets: sockets_string, + sockets: sockets_hash, }.compact end @@ -565,8 +580,6 @@ module Homebrew raise ArgumentError, "Unexpected run command: #{api_hash["run"]}" end - hash[:keep_alive] = api_hash["keep_alive"].transform_keys(&:to_sym) if api_hash.key?("keep_alive") - if api_hash.key?("environment_variables") hash[:environment_variables] = api_hash["environment_variables"].to_h do |key, value| [key.to_sym, replace_placeholders(value)] @@ -585,12 +598,22 @@ module Homebrew hash[key.to_sym] = replace_placeholders(value) end - %w[interval cron launch_only_once require_root restart_delay macos_legacy_timers sockets].each do |key| + %w[interval cron launch_only_once require_root restart_delay macos_legacy_timers].each do |key| next if (value = api_hash[key]).nil? hash[key.to_sym] = value end + %w[sockets keep_alive].each do |key| + next unless (value = api_hash[key]) + + hash[key.to_sym] = if value.is_a?(Hash) + value.transform_keys(&:to_sym) + else + value + end + end + hash end diff --git a/Library/Homebrew/test/service_spec.rb b/Library/Homebrew/test/service_spec.rb index 324a373086..7f28005fb0 100644 --- a/Library/Homebrew/test/service_spec.rb +++ b/Library/Homebrew/test/service_spec.rb @@ -14,6 +14,15 @@ describe Homebrew::Service do end end + def stub_formula_with_service_sockets(sockets_var) + stub_formula do + service do + run opt_bin/"beanstalkd" + sockets sockets_var + end + end + end + describe "#std_service_path_env" do it "returns valid std_service_path_env" do f = stub_formula do @@ -102,43 +111,33 @@ describe Homebrew::Service do end describe "#sockets" do - it "throws for missing type" do - f = stub_formula do - service do - run opt_bin/"beanstalkd" - sockets "127.0.0.1:80" - end - end + let(:sockets_type_error_message) { "Service#sockets a formatted socket definition as ://:" } - expect do - f.service.manual_command - end.to raise_error TypeError, "Service#sockets a formatted socket definition as ://:" + it "throws for missing type" do + [ + stub_formula_with_service_sockets("127.0.0.1:80"), + stub_formula_with_service_sockets({ "Socket" => "127.0.0.1:80" }), + ].each do |f| + expect { f.service.manual_command }.to raise_error TypeError, sockets_type_error_message + end end it "throws for missing host" do - f = stub_formula do - service do - run opt_bin/"beanstalkd" - sockets "tcp://:80" - end + [ + stub_formula_with_service_sockets("tcp://:80"), + stub_formula_with_service_sockets({ "Socket" => "tcp://:80" }), + ].each do |f| + expect { f.service.manual_command }.to raise_error TypeError, sockets_type_error_message end - - expect do - f.service.manual_command - end.to raise_error TypeError, "Service#sockets a formatted socket definition as ://:" end it "throws for missing port" do - f = stub_formula do - service do - run opt_bin/"beanstalkd" - sockets "tcp://127.0.0.1" - end + [ + stub_formula_with_service_sockets("tcp://127.0.0.1"), + stub_formula_with_service_sockets({ "Socket" => "tcp://127.0.0.1" }), + ].each do |f| + expect { f.service.manual_command }.to raise_error TypeError, sockets_type_error_message end - - expect do - f.service.manual_command - end.to raise_error TypeError, "Service#sockets a formatted socket definition as ://:" end end @@ -259,10 +258,59 @@ describe Homebrew::Service do end it "returns valid plist with socket" do + plist_expect = <<~EOS + + + + + \tLabel + \thomebrew.mxcl.formula_name + \tLimitLoadToSessionType + \t + \t\tAqua + \t\tBackground + \t\tLoginWindow + \t\tStandardIO + \t\tSystem + \t + \tProgramArguments + \t + \t\t#{HOMEBREW_PREFIX}/opt/formula_name/bin/beanstalkd + \t + \tRunAtLoad + \t + \tSockets + \t + \t\tlisteners + \t\t + \t\t\tSockFamily + \t\t\tIPv4v6 + \t\t\tSockNodeName + \t\t\t127.0.0.1 + \t\t\tSockProtocol + \t\t\tTCP + \t\t\tSockServiceName + \t\t\t80 + \t\t + \t + + + EOS + + [ + stub_formula_with_service_sockets("tcp://127.0.0.1:80"), + stub_formula_with_service_sockets({ listeners: "tcp://127.0.0.1:80" }), + ].each do |f| + plist = f.service.to_plist + expect(plist).to eq(plist_expect) + end + end + + it "returns valid plist with multiple sockets" do f = stub_formula do service do run [opt_bin/"beanstalkd", "test"] - sockets "tcp://127.0.0.1:80" + sockets socket: "tcp://0.0.0.0:80", socket_tls: "tcp://0.0.0.0:443" end end @@ -291,17 +339,28 @@ describe Homebrew::Service do \t \tSockets \t - \t\tListeners + \t\tsocket \t\t \t\t\tSockFamily \t\t\tIPv4v6 \t\t\tSockNodeName - \t\t\t127.0.0.1 + \t\t\t0.0.0.0 \t\t\tSockProtocol \t\t\tTCP \t\t\tSockServiceName \t\t\t80 \t\t + \t\tsocket_tls + \t\t + \t\t\tSockFamily + \t\t\tIPv4v6 + \t\t\tSockNodeName + \t\t\t0.0.0.0 + \t\t\tSockProtocol + \t\t\tTCP + \t\t\tSockServiceName + \t\t\t443 + \t\t \t @@ -990,7 +1049,7 @@ describe Homebrew::Service do run_type: :immediate, working_dir: "/$HOME", cron: "0 0 * * 0", - sockets: "tcp://0.0.0.0:80", + sockets: { listeners: "tcp://0.0.0.0:80" }, } end diff --git a/docs/Formula-Cookbook.md b/docs/Formula-Cookbook.md index 7255e0bc80..399ebfb461 100644 --- a/docs/Formula-Cookbook.md +++ b/docs/Formula-Cookbook.md @@ -1052,6 +1052,24 @@ The `sockets` method accepts a formatted socket definition as `://:< Please note that sockets will be accessible on IPv4 and IPv6 addresses by default. +If you only need one socket and you don't care about the name (the default is `listeners`): + +```rb +service do + run [opt_bin/"beanstalkd", "test"] + sockets "tcp://127.0.0.1:80" +end +``` + +If you need multiple sockets and/or you want to specify the name: + +```rb +service do + run [opt_bin/"beanstalkd", "test"] + sockets http: "tcp://0.0.0.0:80", https: "tcp://0.0.0.0:443" +end +``` + ### Using environment variables Homebrew has multiple levels of environment variable filtering which affects which variables are available to formulae. From 215419daa505d000d76462e3e8a91316b4a4a218 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Fri, 29 Sep 2023 19:45:55 -0700 Subject: [PATCH 2/2] service: provide backwards compatibility for socket strings The previous PR changed how sockets were represented in the JSON API for formulae and that would cause problems when trying to install packages with service sockets. This provides backwards compatibility until all users have upgraded to versions of homebrew that can deserialize sockets hashes (maybe a couple weeks). Essentially, we store the socket string when serializing sockets that were originally defined with only the string parameter otherwise we serialize it to a hash. --- Library/Homebrew/service.rb | 30 ++++++++++++++++++++------- Library/Homebrew/test/service_spec.rb | 25 +++++++++++++--------- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/Library/Homebrew/service.rb b/Library/Homebrew/service.rb index 3d1de39211..09e2fb6b59 100644 --- a/Library/Homebrew/service.rb +++ b/Library/Homebrew/service.rb @@ -1,6 +1,7 @@ # typed: true # frozen_string_literal: true +require "ipaddr" require "extend/on_system" module Homebrew @@ -187,7 +188,7 @@ module Homebrew end end - SOCKET_STRING_REGEX = %r{([a-z]+)://([a-z0-9.]+):([0-9]+)}i.freeze + SOCKET_STRING_REGEX = %r{^([a-z]+)://(.+):([0-9]+)$}i.freeze sig { params(value: T.nilable(T.any(String, T::Hash[Symbol, String]))) @@ -206,6 +207,13 @@ module Homebrew raise TypeError, "Service#sockets a formatted socket definition as ://:" if match.blank? type, host, port = match.captures + + begin + IPAddr.new(host) + rescue IPAddr::InvalidAddressError + raise TypeError, "Service#sockets expects a valid ipv4 or ipv6 host address" + end + { host: host, port: port, type: type } end end @@ -424,7 +432,6 @@ module Homebrew SockNodeName: info[:host], SockServiceName: info[:port], SockProtocol: info[:type].upcase, - SockFamily: "IPv4v6", } end end @@ -522,10 +529,19 @@ module Homebrew .join(" ") end - sockets_hash = if @sockets.present? - @sockets.transform_values do |info| - "#{info[:type]}://#{info[:host]}:#{info[:port]}" - end + sockets_var = if @sockets.present? + @sockets.transform_values { |info| "#{info[:type]}://#{info[:host]}:#{info[:port]}" } + .then do |sockets_hash| + # TODO: Remove this code when all users are running on versions of Homebrew + # that can process sockets hashes (this commit or later). + if sockets_hash.size == 1 && sockets_hash.key?(:listeners) + # When original #sockets argument was a string: `sockets "tcp://127.0.0.1:80"` + sockets_hash.fetch(:listeners) + else + # When original #sockets argument was a hash: `sockets http: "tcp://0.0.0.0:80"` + sockets_hash + end + end end { @@ -546,7 +562,7 @@ module Homebrew restart_delay: @restart_delay, process_type: @process_type, macos_legacy_timers: @macos_legacy_timers, - sockets: sockets_hash, + sockets: sockets_var, }.compact end diff --git a/Library/Homebrew/test/service_spec.rb b/Library/Homebrew/test/service_spec.rb index 7f28005fb0..85f1e8be51 100644 --- a/Library/Homebrew/test/service_spec.rb +++ b/Library/Homebrew/test/service_spec.rb @@ -116,7 +116,7 @@ describe Homebrew::Service do it "throws for missing type" do [ stub_formula_with_service_sockets("127.0.0.1:80"), - stub_formula_with_service_sockets({ "Socket" => "127.0.0.1:80" }), + stub_formula_with_service_sockets({ socket: "127.0.0.1:80" }), ].each do |f| expect { f.service.manual_command }.to raise_error TypeError, sockets_type_error_message end @@ -125,7 +125,7 @@ describe Homebrew::Service do it "throws for missing host" do [ stub_formula_with_service_sockets("tcp://:80"), - stub_formula_with_service_sockets({ "Socket" => "tcp://:80" }), + stub_formula_with_service_sockets({ socket: "tcp://:80" }), ].each do |f| expect { f.service.manual_command }.to raise_error TypeError, sockets_type_error_message end @@ -134,11 +134,22 @@ describe Homebrew::Service do it "throws for missing port" do [ stub_formula_with_service_sockets("tcp://127.0.0.1"), - stub_formula_with_service_sockets({ "Socket" => "tcp://127.0.0.1" }), + stub_formula_with_service_sockets({ socket: "tcp://127.0.0.1" }), ].each do |f| expect { f.service.manual_command }.to raise_error TypeError, sockets_type_error_message end end + + it "throws for invalid host" do + [ + stub_formula_with_service_sockets("tcp://300.0.0.1:80"), + stub_formula_with_service_sockets({ socket: "tcp://300.0.0.1:80" }), + ].each do |f| + expect do + f.service.manual_command + end.to raise_error TypeError, "Service#sockets expects a valid ipv4 or ipv6 host address" + end + end end describe "#manual_command" do @@ -283,8 +294,6 @@ describe Homebrew::Service do \t \t\tlisteners \t\t - \t\t\tSockFamily - \t\t\tIPv4v6 \t\t\tSockNodeName \t\t\t127.0.0.1 \t\t\tSockProtocol @@ -341,8 +350,6 @@ describe Homebrew::Service do \t \t\tsocket \t\t - \t\t\tSockFamily - \t\t\tIPv4v6 \t\t\tSockNodeName \t\t\t0.0.0.0 \t\t\tSockProtocol @@ -352,8 +359,6 @@ describe Homebrew::Service do \t\t \t\tsocket_tls \t\t - \t\t\tSockFamily - \t\t\tIPv4v6 \t\t\tSockNodeName \t\t\t0.0.0.0 \t\t\tSockProtocol @@ -1049,7 +1054,7 @@ describe Homebrew::Service do run_type: :immediate, working_dir: "/$HOME", cron: "0 0 * * 0", - sockets: { listeners: "tcp://0.0.0.0:80" }, + sockets: "tcp://0.0.0.0:80", } end