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

Kube2IAM should mount /run/xtables.lock #216

Open
yannh opened this issue Jul 12, 2019 · 0 comments
Open

Kube2IAM should mount /run/xtables.lock #216

yannh opened this issue Jul 12, 2019 · 0 comments

Comments

@yannh
Copy link
Contributor

yannh commented Jul 12, 2019

When iptables rules are synced/applied, iptables uses a lock on /run/xtables.lock to ensure that no other process is modifying iptables rules. See for reference discussions kubernetes/kubernetes#7370 or kubernetes-sigs/kubespray#4073

Kube2iam uses the github.com/coreos/go-iptables/iptable library, which does lock using this file, see: https://github.com/coreos/go-iptables/blob/b5b1876b170881a8259f036445ee89c8669db386/iptables/lock.go#L29

However kube2iam does not mount this file from the host - therefore it only locks the file within the container, leading to potential race conditions when applying iptables.
I would suggest mounting the file from the host.

Note: coreos/go-iptables#61 seems worrying. If the lock is never released, that might prevent other critical services from starting up. EDIT: this is actually only for old versions of iptables (<1.4.20).

mariusv pushed a commit to mariusv/kube2iam that referenced this issue Nov 12, 2020
* kube2iam chart

* set hostNetwork at the spec level

* fixes from code review h/t @mgoodness

* cleanup/style

* linter, host needs to be a dictionary

* move kube2iam to stable
jtblin pushed a commit that referenced this issue Nov 13, 2020
* Add kube2iam to control AWS IAM policy access (#216)

* kube2iam chart

* set hostNetwork at the spec level

* fixes from code review h/t @mgoodness

* cleanup/style

* linter, host needs to be a dictionary

* move kube2iam to stable

* kube2iam: Don't quote .Values.host.interface; it breaks iptables wildcard support (#387)

* .Values.host.interface shouldn't be quoted

In some network configurations we have to handle traffic to the metadata api
from multiple interfaces - typically the node side of a veth pair, where the
other side lives inside a Pod's network namespace.  We could use the wildcard
functionality in iptables to do this, but setting `host.interface: veth+` in
this chart doesn't work.

This is because the parameter is quoted in the DaemonSet template and kube2iam
ends up sending the parameter it receives on the command-line directly to
`execvp("iptables", ...)`, which means that the parameter stays quoted all the
way into the actual iptables rule. So you end up with a rule that looks like
this:

```shell
iptables -t nat -S PREROUTING | grep 169.254.169.254
-A PREROUTING -d 169.254.169.254/32 -i "veth+" -p tcp -m tcp --dport 80 -j DNAT --to-destination 10.0.0.1:8181
```

this rule only matches an interface named exactly veth+ since the wildcard
character apparently isn't expanded when the interface name is quoted. If we
remove the quoting we can support iptables wildcards and I don't see why it
shouldn't work for exact matching the interface name still.

* Bump version

* Missing chart in helm install statement (#598)

* Missing chart in helm install statement

Usage: (v2.1.3)
  helm install [CHART] [flags]

* Include chart in install statement
* Fix set flag to use strings as required

* Trim to single line set option

* Managed to stomp the package during rebase

* Consistency of install package

* [stable/kube2iam] v0.2.0 (#615)

* Added .helmignore
* Updated common labels per proposed best practices
* Split image repository and tag
* Don't quote host interface argument (fix)
* Added verbose option
* Docs
* YAML formatting
* Removed default function in favor of values.yaml

* [stable/kube2iam] No default resources (#682)

* Pedantic commenting

* No default resources

* Semver-compliant

* [stable/kube2iam] Rolling updates (#1276)

* [stable/kube2iam] Rolling updates

Also bump to latest image version

* Allow further update strategies

* [stable/kube2iam]: add rbac support (#1286)

* add rbac support

* solve and edge-case when turning off rbac

* Use consistent whitespace in template placeholders (#1437)

Use consistent whitespace in template placeholders

* [stable/kube2iam] fix: update default image version to latest (#1492)

* fix: update default image version to latest

* Bump chart version

* [stable/kube2iam] Allow user-managed RBAC (#1504)

* Allow user-managed RBAC

* Add namespaces to ClusterRole

* [stable/kube2iam] Update strategy (#1510)

* Set UpdateStrategy using .Capabilities

* Bump chart version

* Support Helm release name = chart name (#1588)

* Use latest kube2iam (#1825)

* [stable/kube2iam] Add ability to configure node tolerations (#1829)

* Add podLabels (#1920)

* [stable/kube2iam] #1785 namespace defined templates with chart name (#2132)

* stable/kube2iam: added support for aws access keys (#2675)

Added a secret resource to allow setting AWS access keys and region

* [stable/kube2iam] Bump to version 0.9.0 (#3249)

* [stable/kube2iam] Add liveness probe (#3400)

* Add liveness check so kube takes care of kube2iam in the event of any issues

* Configure --app-port to remain consistent with Helm chart value of .host.port

* Bump minor version

* Remove option to disable liveness probe

* Add support for boolean flags via extraArgs (#3792)

* typo:fix tables to table  (#4346)

* patch3

patch3

* patch-2 tables

patch-2 tables

* Added the possibility to specify affinity options to kube2iam chart (#4203)

* kube2iam update to 0.10.0; fixes 5th generation instance types and cr… (#4535)

* kube2iam update to 0.10.0; fixes 5th generation instance types and cross namespace permissions

* kube2iam adding home to Chart.yaml

* Fix for version comparison from strings to semver (#4600)

* Fix for version comparison from strings to semver

See #3002 for more detail

* Updating maintainers to github ids

* Fixing " error due to wrong " character

* Include pre-releases in the semver ranges

This is important when testing against alpha and beta builds of
Kubernetes along with environments that use pre-releases to denote
things other than pre-releases (e.g., gke denotes the environment
with a pre-releases)

* make liveness probe conditional (#4612)

* Show tolerations options in  README.md (#4830)

* Update README.md

* Incrementing the chart version

* [stable/kube2iam] Support extra container environment variables (#5091)

* Support extra container environment variables.

* [stable/kube2iam] Ensure extra container environment variable values are quoted.

* [kube2iam] Add node flag to limit relevant pods (#5652)

* [kube2iam] Add node flag to limit relevant pods

* Update Chart.yaml

* added missing get verb (#8667)

* added missing get verb

Signed-off-by: Amir Kibbar <[email protected]>

* bumped version

Signed-off-by: Amir Kibbar <[email protected]>

* add priorityClassName, upgrade kube2iam (#9092)

Signed-off-by: Taehyun Kim <[email protected]>

* Adds Prometheus ServiceMonitor resource to kube2iam (#11416)

Changes:
 - Adds ServiceMonitor & Service resources for use with Prometheus Operator.
 - Allows configuring the metrics port option of kube2iam and will
 configure the new named port on the DaemonSet when applicable.
 - Adds docs for all new config params

Bonus:
 - Adds missing docs for `host.port` config param

Signed-off-by: Will Frew <[email protected]>

* add apiVersion (#13801)

Signed-off-by: Carlos Panato <[email protected]>

* [stable/kube2iam] Fix issue when changing kube2iam host port (#13729)

does not change metrics port and requires second custom port

Signed-off-by: George Kaz <[email protected]>

* [stable/kube2iam] Upgrade version, Add prometheus service annotations (#15626)

* Upgrade Kube2iam, Add prometheus service annotations

Signed-off-by: Fabio Todaro <[email protected]>

* Update README

Signed-off-by: Fabio Todaro <[email protected]>

* [stable/kube2iam] Use labels recommended by Helm (#15700)

See https://helm.sh/docs/chart_best_practices/#standard-labels

Since the DaemonSet's selector is immutable, this is breaking change and will require a deletion and recreation, hence the major version bump.

Signed-off-by: Peter Rifel <[email protected]>

* [stable/kube2iam] Update version in documentation (#16092)

* [stable/kube2iam] Update version in documentation

Change version from 0.10.4 to 0.10.7

Signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

* Bump chart version from 2.0.0 to 2.0.1

Signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

* [stable/kube2iam] corrects labels in notes to new format (#16703)

Signed-off-by: Dennis Webb <[email protected]>

* Add in imagePullSecrets config for kube2iam chart (#18815)

Signed-off-by: Benjamin Farley <[email protected]>

* [stable/kube2iam] Update the kube2iam apiVersion for k8s 1.16 compatibility (#18784)

* [stable/kube2iam] Update the kube2iam apiVersion for k8s 1.16 compatibility

Signed-off-by: Mike Tougeron <[email protected]>

* [stable/kube2iam] fix the selector

Signed-off-by: Mike Tougeron <[email protected]>

* [stable/kube2iam] Allow setting a custom secret name to use for AWS credentials (#21265)

* Allow setting a custom secret name to use for AWS credentials in the event that the secret is populated from outside of helm for security purposes

Signed-off-by: Mike Tougeron <[email protected]>

* [stable/kube2iam] call the variable existingSecret

Signed-off-by: Mike Tougeron <[email protected]>

* Bump minor version for new feature

Signed-off-by: Reinhard Nägele <[email protected]>

Co-authored-by: Reinhard Nägele <[email protected]>

* [stable/kube2iam] add podsecuritypolicy object to the chart (#22074)

Signed-off-by: Yannick Kint <[email protected]>

* [stable/kube2iam] bump kube2iam image and appVersion to v0.10.9 (#22333)

kube2iam v0.10.9 supports 1.17.3 due to client-go version upgrades

Signed-off-by: Marius Voila <[email protected]>

* [kube2iam] - add labels to ServiceMonitor (#21605)

Signed-off-by: Alex Williams <[email protected]>

* [stable/kube2iam] allow to customize liveness probe configuration (#22717)

* allow to customise livenessProbe parameters

Signed-off-by: Luigi Tagliamonte <[email protected]>

* bump chart version

Signed-off-by: Luigi Tagliamonte <[email protected]>

* document variables in the README

Signed-off-by: Luigi Tagliamonte <[email protected]>

* [stable/kube2iam] add repo archive notice (#24143)

Signed-off-by: Scott Rigby <[email protected]>

* add kube2iam chart

Signed-off-by: Marius Voila <[email protected]>

* remove maintainers

Signed-off-by: Marius Voila <[email protected]>

Co-authored-by: Michael Haselton <[email protected]>
Co-authored-by: Øyvind Ingebrigtsen Øvergaard <[email protected]>
Co-authored-by: Vlad <[email protected]>
Co-authored-by: Michael Goodness <[email protected]>
Co-authored-by: Chris Knowles <[email protected]>
Co-authored-by: Sam Clinckspoor <[email protected]>
Co-authored-by: Frederic Hemberger <[email protected]>
Co-authored-by: Frode Egeland <[email protected]>
Co-authored-by: Victor Unegbu <[email protected]>
Co-authored-by: Kevin Schumacher <[email protected]>
Co-authored-by: Jesus Rafael Carrillo <[email protected]>
Co-authored-by: Erick Tryzelaar <[email protected]>
Co-authored-by: Dan Gorst <[email protected]>
Co-authored-by: Peter Rifel <[email protected]>
Co-authored-by: yulng <[email protected]>
Co-authored-by: Maxime VISONNEAU <[email protected]>
Co-authored-by: Shane Starcher <[email protected]>
Co-authored-by: Matt Farina <[email protected]>
Co-authored-by: Khris Richardson <[email protected]>
Co-authored-by: Jakob <[email protected]>
Co-authored-by: Kit Ewbank <[email protected]>
Co-authored-by: Daren Desjardins <[email protected]>
Co-authored-by: Amir <[email protected]>
Co-authored-by: Taehyun Kim <[email protected]>
Co-authored-by: Will Frew <[email protected]>
Co-authored-by: Carlos Tadeu Panato Junior <[email protected]>
Co-authored-by: georgekaz <[email protected]>
Co-authored-by: Fabio Todaro <[email protected]>
Co-authored-by: Nicolas Vanheuverzwijn <[email protected]>
Co-authored-by: Dennis Webb <[email protected]>
Co-authored-by: Benjamin Farley <[email protected]>
Co-authored-by: Mike Tougeron <[email protected]>
Co-authored-by: Reinhard Nägele <[email protected]>
Co-authored-by: KYannick <[email protected]>
Co-authored-by: Alex Williams <[email protected]>
Co-authored-by: Luigi Tagliamonte <[email protected]>
Co-authored-by: Scott Rigby <[email protected]>
jtblin added a commit that referenced this issue Dec 21, 2020
* Add kube2iam to control AWS IAM policy access (#216)

* kube2iam chart

* set hostNetwork at the spec level

* fixes from code review h/t @mgoodness

* cleanup/style

* linter, host needs to be a dictionary

* move kube2iam to stable

* kube2iam: Don't quote .Values.host.interface; it breaks iptables wildcard support (#387)

* .Values.host.interface shouldn't be quoted

In some network configurations we have to handle traffic to the metadata api
from multiple interfaces - typically the node side of a veth pair, where the
other side lives inside a Pod's network namespace.  We could use the wildcard
functionality in iptables to do this, but setting `host.interface: veth+` in
this chart doesn't work.

This is because the parameter is quoted in the DaemonSet template and kube2iam
ends up sending the parameter it receives on the command-line directly to
`execvp("iptables", ...)`, which means that the parameter stays quoted all the
way into the actual iptables rule. So you end up with a rule that looks like
this:

```shell
iptables -t nat -S PREROUTING | grep 169.254.169.254
-A PREROUTING -d 169.254.169.254/32 -i "veth+" -p tcp -m tcp --dport 80 -j DNAT --to-destination 10.0.0.1:8181
```

this rule only matches an interface named exactly veth+ since the wildcard
character apparently isn't expanded when the interface name is quoted. If we
remove the quoting we can support iptables wildcards and I don't see why it
shouldn't work for exact matching the interface name still.

* Bump version

* Missing chart in helm install statement (#598)

* Missing chart in helm install statement

Usage: (v2.1.3)
  helm install [CHART] [flags]

* Include chart in install statement
* Fix set flag to use strings as required

* Trim to single line set option

* Managed to stomp the package during rebase

* Consistency of install package

* [stable/kube2iam] v0.2.0 (#615)

* Added .helmignore
* Updated common labels per proposed best practices
* Split image repository and tag
* Don't quote host interface argument (fix)
* Added verbose option
* Docs
* YAML formatting
* Removed default function in favor of values.yaml

* [stable/kube2iam] No default resources (#682)

* Pedantic commenting

* No default resources

* Semver-compliant

* [stable/kube2iam] Rolling updates (#1276)

* [stable/kube2iam] Rolling updates

Also bump to latest image version

* Allow further update strategies

* [stable/kube2iam]: add rbac support (#1286)

* add rbac support

* solve and edge-case when turning off rbac

* Use consistent whitespace in template placeholders (#1437)

Use consistent whitespace in template placeholders

* [stable/kube2iam] fix: update default image version to latest (#1492)

* fix: update default image version to latest

* Bump chart version

* [stable/kube2iam] Allow user-managed RBAC (#1504)

* Allow user-managed RBAC

* Add namespaces to ClusterRole

* [stable/kube2iam] Update strategy (#1510)

* Set UpdateStrategy using .Capabilities

* Bump chart version

* Support Helm release name = chart name (#1588)

* Use latest kube2iam (#1825)

* [stable/kube2iam] Add ability to configure node tolerations (#1829)

* Add podLabels (#1920)

* [stable/kube2iam] #1785 namespace defined templates with chart name (#2132)

* stable/kube2iam: added support for aws access keys (#2675)

Added a secret resource to allow setting AWS access keys and region

* [stable/kube2iam] Bump to version 0.9.0 (#3249)

* [stable/kube2iam] Add liveness probe (#3400)

* Add liveness check so kube takes care of kube2iam in the event of any issues

* Configure --app-port to remain consistent with Helm chart value of .host.port

* Bump minor version

* Remove option to disable liveness probe

* Add support for boolean flags via extraArgs (#3792)

* typo:fix tables to table  (#4346)

* patch3

patch3

* patch-2 tables

patch-2 tables

* Added the possibility to specify affinity options to kube2iam chart (#4203)

* kube2iam update to 0.10.0; fixes 5th generation instance types and cr… (#4535)

* kube2iam update to 0.10.0; fixes 5th generation instance types and cross namespace permissions

* kube2iam adding home to Chart.yaml

* Fix for version comparison from strings to semver (#4600)

* Fix for version comparison from strings to semver

See #3002 for more detail

* Updating maintainers to github ids

* Fixing " error due to wrong " character

* Include pre-releases in the semver ranges

This is important when testing against alpha and beta builds of
Kubernetes along with environments that use pre-releases to denote
things other than pre-releases (e.g., gke denotes the environment
with a pre-releases)

* make liveness probe conditional (#4612)

* Show tolerations options in  README.md (#4830)

* Update README.md

* Incrementing the chart version

* [stable/kube2iam] Support extra container environment variables (#5091)

* Support extra container environment variables.

* [stable/kube2iam] Ensure extra container environment variable values are quoted.

* [kube2iam] Add node flag to limit relevant pods (#5652)

* [kube2iam] Add node flag to limit relevant pods

* Update Chart.yaml

* added missing get verb (#8667)

* added missing get verb

Signed-off-by: Amir Kibbar <[email protected]>

* bumped version

Signed-off-by: Amir Kibbar <[email protected]>

* add priorityClassName, upgrade kube2iam (#9092)

Signed-off-by: Taehyun Kim <[email protected]>

* Adds Prometheus ServiceMonitor resource to kube2iam (#11416)

Changes:
 - Adds ServiceMonitor & Service resources for use with Prometheus Operator.
 - Allows configuring the metrics port option of kube2iam and will
 configure the new named port on the DaemonSet when applicable.
 - Adds docs for all new config params

Bonus:
 - Adds missing docs for `host.port` config param

Signed-off-by: Will Frew <[email protected]>

* add apiVersion (#13801)

Signed-off-by: Carlos Panato <[email protected]>

* [stable/kube2iam] Fix issue when changing kube2iam host port (#13729)

does not change metrics port and requires second custom port

Signed-off-by: George Kaz <[email protected]>

* [stable/kube2iam] Upgrade version, Add prometheus service annotations (#15626)

* Upgrade Kube2iam, Add prometheus service annotations

Signed-off-by: Fabio Todaro <[email protected]>

* Update README

Signed-off-by: Fabio Todaro <[email protected]>

* [stable/kube2iam] Use labels recommended by Helm (#15700)

See https://helm.sh/docs/chart_best_practices/#standard-labels

Since the DaemonSet's selector is immutable, this is breaking change and will require a deletion and recreation, hence the major version bump.

Signed-off-by: Peter Rifel <[email protected]>

* [stable/kube2iam] Update version in documentation (#16092)

* [stable/kube2iam] Update version in documentation

Change version from 0.10.4 to 0.10.7

Signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

* Bump chart version from 2.0.0 to 2.0.1

Signed-off-by: Nicolas Vanheuverzwijn <[email protected]>

* [stable/kube2iam] corrects labels in notes to new format (#16703)

Signed-off-by: Dennis Webb <[email protected]>

* Add in imagePullSecrets config for kube2iam chart (#18815)

Signed-off-by: Benjamin Farley <[email protected]>

* [stable/kube2iam] Update the kube2iam apiVersion for k8s 1.16 compatibility (#18784)

* [stable/kube2iam] Update the kube2iam apiVersion for k8s 1.16 compatibility

Signed-off-by: Mike Tougeron <[email protected]>

* [stable/kube2iam] fix the selector

Signed-off-by: Mike Tougeron <[email protected]>

* [stable/kube2iam] Allow setting a custom secret name to use for AWS credentials (#21265)

* Allow setting a custom secret name to use for AWS credentials in the event that the secret is populated from outside of helm for security purposes

Signed-off-by: Mike Tougeron <[email protected]>

* [stable/kube2iam] call the variable existingSecret

Signed-off-by: Mike Tougeron <[email protected]>

* Bump minor version for new feature

Signed-off-by: Reinhard Nägele <[email protected]>

Co-authored-by: Reinhard Nägele <[email protected]>

* [stable/kube2iam] add podsecuritypolicy object to the chart (#22074)

Signed-off-by: Yannick Kint <[email protected]>

* [stable/kube2iam] bump kube2iam image and appVersion to v0.10.9 (#22333)

kube2iam v0.10.9 supports 1.17.3 due to client-go version upgrades

Signed-off-by: Marius Voila <[email protected]>

* [kube2iam] - add labels to ServiceMonitor (#21605)

Signed-off-by: Alex Williams <[email protected]>

* [stable/kube2iam] allow to customize liveness probe configuration (#22717)

* allow to customise livenessProbe parameters

Signed-off-by: Luigi Tagliamonte <[email protected]>

* bump chart version

Signed-off-by: Luigi Tagliamonte <[email protected]>

* document variables in the README

Signed-off-by: Luigi Tagliamonte <[email protected]>

* [stable/kube2iam] add repo archive notice (#24143)

Signed-off-by: Scott Rigby <[email protected]>

* fix README

fixes #288

Signed-off-by: Marius Voila <[email protected]>

* Update README.md

Co-authored-by: Michael Haselton <[email protected]>
Co-authored-by: Øyvind Ingebrigtsen Øvergaard <[email protected]>
Co-authored-by: Vlad <[email protected]>
Co-authored-by: Michael Goodness <[email protected]>
Co-authored-by: Chris Knowles <[email protected]>
Co-authored-by: Sam Clinckspoor <[email protected]>
Co-authored-by: Frederic Hemberger <[email protected]>
Co-authored-by: Frode Egeland <[email protected]>
Co-authored-by: Victor Unegbu <[email protected]>
Co-authored-by: Kevin Schumacher <[email protected]>
Co-authored-by: Jesus Rafael Carrillo <[email protected]>
Co-authored-by: Erick Tryzelaar <[email protected]>
Co-authored-by: Dan Gorst <[email protected]>
Co-authored-by: Peter Rifel <[email protected]>
Co-authored-by: yulng <[email protected]>
Co-authored-by: Maxime VISONNEAU <[email protected]>
Co-authored-by: Shane Starcher <[email protected]>
Co-authored-by: Matt Farina <[email protected]>
Co-authored-by: Khris Richardson <[email protected]>
Co-authored-by: Jakob <[email protected]>
Co-authored-by: Kit Ewbank <[email protected]>
Co-authored-by: Daren Desjardins <[email protected]>
Co-authored-by: Amir <[email protected]>
Co-authored-by: Taehyun Kim <[email protected]>
Co-authored-by: Will Frew <[email protected]>
Co-authored-by: Carlos Tadeu Panato Junior <[email protected]>
Co-authored-by: georgekaz <[email protected]>
Co-authored-by: Fabio Todaro <[email protected]>
Co-authored-by: Nicolas Vanheuverzwijn <[email protected]>
Co-authored-by: Dennis Webb <[email protected]>
Co-authored-by: Benjamin Farley <[email protected]>
Co-authored-by: Mike Tougeron <[email protected]>
Co-authored-by: Reinhard Nägele <[email protected]>
Co-authored-by: KYannick <[email protected]>
Co-authored-by: Alex Williams <[email protected]>
Co-authored-by: Luigi Tagliamonte <[email protected]>
Co-authored-by: Scott Rigby <[email protected]>
Co-authored-by: Jerome Touffe-Blin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant