Skip to content

Commit

Permalink
Merge pull request #5 from mruoss/remove-insecure-skip-tls-verify
Browse files Browse the repository at this point in the history
remove `insecure_skip_tls_verify` option and use a custom `match_fun` instead
  • Loading branch information
mruoss authored Dec 19, 2023
2 parents 9e42846 + 27652dc commit b62c02f
Show file tree
Hide file tree
Showing 7 changed files with 251 additions and 20 deletions.
216 changes: 216 additions & 0 deletions .credo.exs
Original file line number Diff line number Diff line change
@@ -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 <name>`. 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`.
#
]
}
}
]
}
3 changes: 2 additions & 1 deletion .tool-versions
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
erlang 26.2.1
elixir 1.15.7
elixir 1.15.7
kind 0.20.0
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

<!-- ### Added | Changed | Deprecated | Removed | Fixed | Security -->

### 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)

<!--------------------- Don't add new entries after this line --------------------->

## [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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 4 additions & 7 deletions lib/flame_k8s_backend.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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} ->
Expand Down Expand Up @@ -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)
Expand Down
31 changes: 22 additions & 9 deletions lib/flame_k8s_backend/k8s_client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,11 @@ 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")

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")
apiserver_port = System.get_env("KUBERNETES_SERVICE_PORT_HTTPS")

with {:ok, token} <- File.read(token_path),
{:ok, ca_cert_raw} <- File.read(ca_cert_path),
Expand All @@ -24,7 +18,12 @@ 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],
customize_hostname_check: [match_fun: &check_ips_as_dns_id/2]
]
]
)
|> Req.Request.append_response_steps(verify_2xs: &verify_2xs/1)

Expand Down Expand Up @@ -87,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
2 changes: 1 addition & 1 deletion test_support/integration_test_runner.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
}]

Expand Down

0 comments on commit b62c02f

Please sign in to comment.