From ea2afb0942195dae71a0d4c9311d0012a05d3e96 Mon Sep 17 00:00:00 2001 From: Michael Ruoss Date: Sat, 16 Dec 2023 13:35:13 +0100 Subject: [PATCH 1/2] replace insecure_skip_tls_verify with auto set sni --- .credo.exs | 216 ++++++++++++++++++++++++ .tool-versions | 3 +- README.md | 2 +- lib/flame_k8s_backend.ex | 11 +- lib/flame_k8s_backend/k8s_client.ex | 24 ++- test_support/integration_test_runner.ex | 2 +- 6 files changed, 239 insertions(+), 19 deletions(-) create mode 100644 .credo.exs diff --git a/.credo.exs b/.credo.exs new file mode 100644 index 0000000..da31610 --- /dev/null +++ b/.credo.exs @@ -0,0 +1,216 @@ +# This file contains the configuration for Credo and you are probably reading +# this after creating it with `mix credo.gen.config`. +# +# If you find anything wrong or unclear in this file, please report an +# issue on GitHub: https://github.com/rrrene/credo/issues +# +%{ + # + # You can have as many configs as you like in the `configs:` field. + configs: [ + %{ + # + # Run any config using `mix credo -C `. If no config name is given + # "default" is used. + # + name: "default", + # + # These are the files included in the analysis: + files: %{ + # + # You can give explicit globs or simply directories. + # In the latter case `**/*.{ex,exs}` will be used. + # + included: [ + "lib/", + "src/", + "test/", + "web/", + "apps/*/lib/", + "apps/*/src/", + "apps/*/test/", + "apps/*/web/" + ], + excluded: [~r"/_build/", ~r"/deps/", ~r"/node_modules/"] + }, + # + # Load and configure plugins here: + # + plugins: [], + # + # If you create your own checks, you must specify the source files for + # them here, so they can be loaded by Credo before running the analysis. + # + requires: [], + # + # If you want to enforce a style guide and need a more traditional linting + # experience, you can change `strict` to `true` below: + # + strict: false, + # + # To modify the timeout for parsing files, change this value: + # + parse_timeout: 5000, + # + # If you want to use uncolored output by default, you can change `color` + # to `false` below: + # + color: true, + # + # You can customize the parameters of any check by adding a second element + # to the tuple. + # + # To disable a check put `false` as second element: + # + # {Credo.Check.Design.DuplicatedCode, false} + # + checks: %{ + enabled: [ + # + ## Consistency Checks + # + {Credo.Check.Consistency.ExceptionNames, []}, + {Credo.Check.Consistency.LineEndings, []}, + {Credo.Check.Consistency.ParameterPatternMatching, []}, + {Credo.Check.Consistency.SpaceAroundOperators, []}, + {Credo.Check.Consistency.SpaceInParentheses, []}, + {Credo.Check.Consistency.TabsOrSpaces, []}, + + # + ## Design Checks + # + # You can customize the priority of any check + # Priority values are: `low, normal, high, higher` + # + {Credo.Check.Design.AliasUsage, + [priority: :low, if_nested_deeper_than: 2, if_called_more_often_than: 0]}, + {Credo.Check.Design.TagFIXME, []}, + # You can also customize the exit_status of each check. + # If you don't want TODO comments to cause `mix credo` to fail, just + # set this value to 0 (zero). + # + {Credo.Check.Design.TagTODO, [exit_status: 2]}, + + # + ## Readability Checks + # + {Credo.Check.Readability.AliasOrder, []}, + {Credo.Check.Readability.FunctionNames, []}, + {Credo.Check.Readability.LargeNumbers, []}, + {Credo.Check.Readability.MaxLineLength, [priority: :low, max_length: 120]}, + {Credo.Check.Readability.ModuleAttributeNames, []}, + {Credo.Check.Readability.ModuleDoc, []}, + {Credo.Check.Readability.ModuleNames, []}, + {Credo.Check.Readability.ParenthesesInCondition, []}, + {Credo.Check.Readability.ParenthesesOnZeroArityDefs, [parens: true]}, + {Credo.Check.Readability.PipeIntoAnonymousFunctions, []}, + {Credo.Check.Readability.PredicateFunctionNames, []}, + {Credo.Check.Readability.PreferImplicitTry, []}, + {Credo.Check.Readability.RedundantBlankLines, []}, + {Credo.Check.Readability.Semicolons, []}, + {Credo.Check.Readability.SpaceAfterCommas, []}, + {Credo.Check.Readability.StringSigils, []}, + {Credo.Check.Readability.TrailingBlankLine, []}, + {Credo.Check.Readability.TrailingWhiteSpace, []}, + {Credo.Check.Readability.UnnecessaryAliasExpansion, []}, + {Credo.Check.Readability.VariableNames, []}, + {Credo.Check.Readability.WithSingleClause, []}, + + # + ## Refactoring Opportunities + # + {Credo.Check.Refactor.Apply, []}, + {Credo.Check.Refactor.CondStatements, []}, + {Credo.Check.Refactor.CyclomaticComplexity, []}, + {Credo.Check.Refactor.FilterCount, []}, + {Credo.Check.Refactor.FilterFilter, []}, + {Credo.Check.Refactor.FunctionArity, []}, + {Credo.Check.Refactor.LongQuoteBlocks, []}, + {Credo.Check.Refactor.MapJoin, []}, + {Credo.Check.Refactor.MatchInCondition, []}, + {Credo.Check.Refactor.NegatedConditionsInUnless, []}, + {Credo.Check.Refactor.NegatedConditionsWithElse, []}, + {Credo.Check.Refactor.Nesting, []}, + {Credo.Check.Refactor.RedundantWithClauseResult, []}, + {Credo.Check.Refactor.RejectReject, []}, + {Credo.Check.Refactor.UnlessWithElse, []}, + {Credo.Check.Refactor.WithClauses, []}, + + # + ## Warnings + # + {Credo.Check.Warning.ApplicationConfigInModuleAttribute, []}, + {Credo.Check.Warning.BoolOperationOnSameValues, []}, + {Credo.Check.Warning.Dbg, []}, + {Credo.Check.Warning.ExpensiveEmptyEnumCheck, []}, + {Credo.Check.Warning.IExPry, []}, + {Credo.Check.Warning.IoInspect, []}, + {Credo.Check.Warning.MissedMetadataKeyInLoggerConfig, []}, + {Credo.Check.Warning.OperationOnSameValues, []}, + {Credo.Check.Warning.OperationWithConstantResult, []}, + {Credo.Check.Warning.RaiseInsideRescue, []}, + {Credo.Check.Warning.SpecWithStruct, []}, + {Credo.Check.Warning.UnsafeExec, []}, + {Credo.Check.Warning.UnusedEnumOperation, []}, + {Credo.Check.Warning.UnusedFileOperation, []}, + {Credo.Check.Warning.UnusedKeywordOperation, []}, + {Credo.Check.Warning.UnusedListOperation, []}, + {Credo.Check.Warning.UnusedPathOperation, []}, + {Credo.Check.Warning.UnusedRegexOperation, []}, + {Credo.Check.Warning.UnusedStringOperation, []}, + {Credo.Check.Warning.UnusedTupleOperation, []}, + {Credo.Check.Warning.WrongTestFileExtension, []} + ], + disabled: [ + # + # Checks scheduled for next check update (opt-in for now, just replace `false` with `[]`) + + # + # Controversial and experimental checks (opt-in, just move the check to `:enabled` + # and be sure to use `mix credo --strict` to see low priority checks) + # + {Credo.Check.Consistency.MultiAliasImportRequireUse, []}, + {Credo.Check.Consistency.UnusedVariableNames, []}, + {Credo.Check.Design.DuplicatedCode, []}, + {Credo.Check.Design.SkipTestWithoutComment, []}, + {Credo.Check.Readability.AliasAs, []}, + {Credo.Check.Readability.BlockPipe, []}, + {Credo.Check.Readability.ImplTrue, []}, + {Credo.Check.Readability.MultiAlias, []}, + {Credo.Check.Readability.NestedFunctionCalls, []}, + {Credo.Check.Readability.OneArityFunctionInPipe, []}, + {Credo.Check.Readability.OnePipePerLine, []}, + {Credo.Check.Readability.SeparateAliasRequire, []}, + {Credo.Check.Readability.SingleFunctionToBlockPipe, []}, + {Credo.Check.Readability.SinglePipe, []}, + {Credo.Check.Readability.Specs, []}, + {Credo.Check.Readability.StrictModuleLayout, []}, + {Credo.Check.Readability.WithCustomTaggedTuple, []}, + {Credo.Check.Refactor.ABCSize, []}, + {Credo.Check.Refactor.AppendSingleItem, []}, + {Credo.Check.Refactor.DoubleBooleanNegation, []}, + {Credo.Check.Refactor.FilterReject, []}, + {Credo.Check.Refactor.IoPuts, []}, + {Credo.Check.Refactor.MapMap, []}, + {Credo.Check.Refactor.ModuleDependencies, []}, + {Credo.Check.Refactor.NegatedIsNil, []}, + {Credo.Check.Refactor.PassAsyncInTestCases, []}, + {Credo.Check.Refactor.PipeChainStart, []}, + {Credo.Check.Refactor.RejectFilter, []}, + {Credo.Check.Refactor.VariableRebinding, []}, + {Credo.Check.Warning.LazyLogging, []}, + {Credo.Check.Warning.LeakyEnvironment, []}, + {Credo.Check.Warning.MapGetUnsafePass, []}, + {Credo.Check.Warning.MixEnv, []}, + {Credo.Check.Warning.UnsafeToAtom, []} + + # {Credo.Check.Refactor.MapInto, []}, + + # + # Custom checks can be created using `mix credo.gen.check`. + # + ] + } + } + ] +} diff --git a/.tool-versions b/.tool-versions index 39066e5..5581251 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1,2 +1,3 @@ erlang 26.2.1 -elixir 1.15.7 \ No newline at end of file +elixir 1.15.7 +kind 0.20.0 diff --git a/README.md b/README.md index f49cb74..9e43ddc 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ Configure the flame backend in our configuration or application setup: children = [ {FLAME.Pool, name: MyApp.SamplePool, - backend: {FLAMEK8sBackend, insecure_skip_tls_verify: true}, + backend: FLAMEK8sBackend, min: 0, max: 10, max_concurrency: 5, diff --git a/lib/flame_k8s_backend.ex b/lib/flame_k8s_backend.ex index 10f22d8..3b882b6 100644 --- a/lib/flame_k8s_backend.ex +++ b/lib/flame_k8s_backend.ex @@ -46,9 +46,6 @@ defmodule FLAMEK8sBackend do * `:log` - The log level to use for verbose logging. Defaults to `false`. - * `:insecure_skip_tls_verify` - Skip TLS hostname verification. This is - required if you connect to your cluster via IP address. - ### Prerequisites In order for this to work, your application needs to meet some requirements. @@ -172,7 +169,7 @@ defmodule FLAMEK8sBackend do log: false, req: nil - @valid_opts ~w(app_container_name insecure_skip_tls_verify runner_pod_tpl terminator_sup log)a + @valid_opts ~w(app_container_name runner_pod_tpl terminator_sup log)a @required_config ~w()a @impl true @@ -205,7 +202,7 @@ defmodule FLAMEK8sBackend do |> FLAME.Parent.new(self(), __MODULE__) |> FLAME.Parent.encode() - {:ok, req} = K8sClient.connect(Keyword.take(provided_opts, [:insecure_skip_tls_verify])) + {:ok, req} = K8sClient.connect() case K8sClient.get_pod(req, System.get_env("POD_NAMESPACE"), System.get_env("POD_NAME")) do {:ok, base_pod} -> @@ -248,9 +245,9 @@ defmodule FLAMEK8sBackend do end @impl true - def system_shutdown do + def system_shutdown() do # This is not very nice but I don't have the opts on the runner - {:ok, req} = K8sClient.connect(insecure_skip_tls_verify: true) + {:ok, req} = K8sClient.connect() namespace = System.get_env("POD_NAMESPACE") name = System.get_env("POD_NAME") K8sClient.delete_pod!(req, namespace, name) diff --git a/lib/flame_k8s_backend/k8s_client.ex b/lib/flame_k8s_backend/k8s_client.ex index 97dff34..a73fafe 100644 --- a/lib/flame_k8s_backend/k8s_client.ex +++ b/lib/flame_k8s_backend/k8s_client.ex @@ -5,17 +5,17 @@ defmodule FLAMEK8sBackend.K8sClient do @pod_tpl "/api/v1/namespaces/:namespace/pods/:name" @pod_list_tpl "/api/v1/namespaces/:namespace/pods" - def connect(opts) do + def connect() do ca_cert_path = Path.join(@sa_token_path, "ca.crt") token_path = Path.join(@sa_token_path, "token") + apiserver_host = System.get_env("KUBERNETES_SERVICE_HOST") |> String.to_charlist() + apiserver_port = System.get_env("KUBERNETES_SERVICE_PORT_HTTPS") - verify = - if Keyword.get(opts, :insecure_skip_tls_verify, false), - do: :verify_none, - else: :verify_peer - - apiserver_host = System.get_env("KUBERNETES_SERVICE_HOST") - apiserver_port = System.get_env("KUBERNETES_SERVICE_PORT") + sni = + case :inet.parse_address(apiserver_host) do + {:ok, _} -> :disable + {:error, _} -> apiserver_host + end with {:ok, token} <- File.read(token_path), {:ok, ca_cert_raw} <- File.read(ca_cert_path), @@ -24,7 +24,13 @@ defmodule FLAMEK8sBackend.K8sClient do Req.new( base_url: "https://#{apiserver_host}:#{apiserver_port}", headers: [{:Authorization, "Bearer #{token}"}], - connect_options: [transport_opts: [cacerts: [ca_cert], verify: verify]] + connect_options: [ + transport_opts: [ + cacerts: [ca_cert], + verify: :verify_peer, + server_name_indication: sni + ] + ] ) |> Req.Request.append_response_steps(verify_2xs: &verify_2xs/1) diff --git a/test_support/integration_test_runner.ex b/test_support/integration_test_runner.ex index a063792..34d76e6 100644 --- a/test_support/integration_test_runner.ex +++ b/test_support/integration_test_runner.ex @@ -10,7 +10,7 @@ defmodule FlameK8sBackend.IntegrationTestRunner do min: 0, max: 2, idle_shutdown_after: 1_000, - backend: {FLAMEK8sBackend, insecure_skip_tls_verify: true}, + backend: FLAMEK8sBackend, log: :debug }] From 27652dc7a112e3c0fd2c6848afba15223c40a6a2 Mon Sep 17 00:00:00 2001 From: Michael Ruoss Date: Mon, 18 Dec 2023 16:45:28 +0100 Subject: [PATCH 2/2] use custom hostname_check to match ips as dns_id --- CHANGELOG.md | 6 +++++- lib/flame_k8s_backend/k8s_client.ex | 25 ++++++++++++++++--------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65175b4..79749eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,13 +10,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +### Changed + +- Remove`:insecure_skip_tls_verify` option and use a custom `match_fun` instead to work around failing hostname verification for IP addresses. - [#5](https://github.com/mruoss/flame_k8s_backend/pull/5) + ## [0.2.3] - 2023-12-15 ### Added -- `runner_pod_tpl` option for better control over the runner pod manifest [#2](https://github.com/mruoss/flame_k8s_backend/pull/2) +- `runner_pod_tpl` option for better control over the runner pod manifest - [#2](https://github.com/mruoss/flame_k8s_backend/pull/2) - Basic integration test ### Changed diff --git a/lib/flame_k8s_backend/k8s_client.ex b/lib/flame_k8s_backend/k8s_client.ex index a73fafe..0adeda4 100644 --- a/lib/flame_k8s_backend/k8s_client.ex +++ b/lib/flame_k8s_backend/k8s_client.ex @@ -8,15 +8,9 @@ defmodule FLAMEK8sBackend.K8sClient do def connect() do ca_cert_path = Path.join(@sa_token_path, "ca.crt") token_path = Path.join(@sa_token_path, "token") - apiserver_host = System.get_env("KUBERNETES_SERVICE_HOST") |> String.to_charlist() + apiserver_host = System.get_env("KUBERNETES_SERVICE_HOST") apiserver_port = System.get_env("KUBERNETES_SERVICE_PORT_HTTPS") - sni = - case :inet.parse_address(apiserver_host) do - {:ok, _} -> :disable - {:error, _} -> apiserver_host - end - with {:ok, token} <- File.read(token_path), {:ok, ca_cert_raw} <- File.read(ca_cert_path), {:ok, ca_cert} <- cert_from_pem(ca_cert_raw) do @@ -27,8 +21,7 @@ defmodule FLAMEK8sBackend.K8sClient do connect_options: [ transport_opts: [ cacerts: [ca_cert], - verify: :verify_peer, - server_name_indication: sni + customize_hostname_check: [match_fun: &check_ips_as_dns_id/2] ] ] ) @@ -93,4 +86,18 @@ defmodule FLAMEK8sBackend.K8sClient do {request, RuntimeError.exception(response.body["message"])} end end + + # Temporary workaround until this is fixed in some lower layer + # https://github.com/erlang/otp/issues/7968 + # https://github.com/elixir-mint/mint/pull/418 + defp check_ips_as_dns_id({:dns_id, hostname}, {:iPAddress, ip}) do + with {:ok, ip_tuple} <- :inet.parse_address(hostname), + ^ip <- Tuple.to_list(ip_tuple) do + true + else + _ -> :default + end + end + + defp check_ips_as_dns_id(_, _), do: :default end