-
Notifications
You must be signed in to change notification settings - Fork 187
Conversation
1d5bea1
to
faaec22
Compare
👆 rebased on the |
🎩 Manually tested with a file and a cluster with the
|
// Ignore warnings from kubeclient as they are expected to be reported by the deprecatedapi auditor. | ||
kubeconfig.WarningHandler = rest.NoWarnings{} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will hide warning messages such as the following:
W0608 12:04:51.010357 30221 warnings.go:70] v1 ComponentStatus is deprecated in v1.19+
W0608 12:04:54.355517 30221 warnings.go:70] extensions/v1beta1 Ingress is deprecated in v1.14+, unavailable in v1.22+; use networking.k8s.io/v1 Ingress
Kubeaudit has to use the deprecated APIs to include them in the deprecatedapis reports.
See https://kubernetes.io/blog/2020/09/03/warnings/#customize-client-handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No major concerns though I would like to put this in the next release, so will approve / merge when the current release it out!
README.md
Outdated
| `apparmor` | Finds containers running without AppArmor. | [docs](docs/auditors/apparmor.md) | | ||
| `asat` | Finds pods using an automatically mounted default service account | [docs](docs/auditors/asat.md) | | ||
| `capabilities` | Finds containers that do not drop the recommended capabilities or add new ones. | [docs](docs/auditors/capabilities.md) | | ||
| `deprecatedapis` | Finds containers that do not drop the recommended capabilities or add new ones. | [docs](docs/auditors/deprecatedapis.md) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description looks copy/pasted from the capabilities auditor
Finds containers that do not drop the recommended capabilities or add new ones.
}, nil | ||
} | ||
|
||
type apiLifecycleDeprecated interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we maybe add a comment with a reference link for these functions? I assume they're defined somewhere in the kubernetes codebase?
APILifecycleReplacement() schema.GroupVersionKind | ||
} | ||
|
||
type apiLifecycleIntroduced interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this interface used anywhere?
APILifecycleIntroduced() (major, minor int) | ||
} | ||
|
||
// Audit checks that the resource API version is not deprecetated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Audit checks that the resource API version is not deprecetated | |
// Audit checks that the resource API version is not deprecated |
var auditResults []*kubeaudit.AuditResult | ||
lastApplied, ok := k8s.GetAnnotations(resource)[v1.LastAppliedConfigAnnotation] | ||
if ok && len(lastApplied) > 0 { | ||
resource, _ = k8sinternal.DecodeResource([]byte(lastApplied)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we ignoring this error?
docs/auditors/deprecatedapis.md
Outdated
@@ -0,0 +1,96 @@ | |||
# Kubernetes Deprecated API Auditor (deprecatedapis) | |||
|
|||
Finds any resource defined with adeprecated API version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finds any resource defined with adeprecated API version. | |
Finds any resource defined with a deprecated API version. |
docs/auditors/deprecatedapis.md
Outdated
|
||
## Examples | ||
|
||
The `deprecatedapis` auditor allows to find the deprecated APIs in use and indicates the versions where they will be removed and replacement APIs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this sentence is easier to understand if it's broken up a bit
The `deprecatedapis` auditor allows to find the deprecated APIs in use and indicates the versions where they will be removed and replacement APIs. | |
The `deprecatedapis` auditor finds the deprecated APIs in use, reports the versions where they will be removed, and recommends replacement APIs. |
docs/auditors/deprecatedapis.md
Outdated
ReplacementGroup: batch/v1 | ||
``` | ||
|
||
The `deprecatedapis` auditor can be used `--targeted-k8s-version` flag. If the API is not yet deprecated for this version the auditor will produce an `info` otherwise a `warning`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `deprecatedapis` auditor can be used `--targeted-k8s-version` flag. If the API is not yet deprecated for this version the auditor will produce an `info` otherwise a `warning`. | |
The `deprecatedapis` auditor can be used with the `--targeted-k8s-version` flag. If the API is not yet deprecated for this version the auditor will produce an `info` otherwise a `warning`. |
docs/auditors/deprecatedapis.md
Outdated
ReplacementGroup: batch/v1 | ||
``` | ||
|
||
The `deprecatedapis` auditor can be used `--targeted-k8s-version` flag. If the API is not available for the targeted version the auditor will produce an `error` otherwise a `warning` or `info` if the API is not yet deprecated for this version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `deprecatedapis` auditor can be used `--targeted-k8s-version` flag. If the API is not available for the targeted version the auditor will produce an `error` otherwise a `warning` or `info` if the API is not yet deprecated for this version. | |
The `deprecatedapis` auditor can be used with the `--targeted-k8s-version` flag. If the API is not available for the targeted version the auditor will produce an `error` otherwise a `warning` or `info` if the API is not yet deprecated for this version. |
| | --current-k8s-version | Kubernetes current version | | | ||
| | --targeted-k8s-version | Kubernetes version to migrate to | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit unclear why there is a flag for both current and target version. Maybe I'm misunderstanding the use case, but to me it sounds easier to reason about if we had just the target version:
ERROR
if the resource API is removed in the target versionWARN
if the resource API is deprecated in the target versionINFO
if the resource API is not deprecated in the target version but will be deprecated in a future version
We could default to the current version in local and cluster mode since kubeaudit should be able to look that up automatically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we specify a current version
or this information is retrieved from the cluster without a targeted version we can have the following cases:
ERROR
if the resource API is removed in the current versionWARN
if the resource API is deprecated in the current versionINFO
if the resource API is not deprecated in the current version but will be deprecated in a future version
If we use a targeted version the message level takes precedence over the current version.
But maybe we can remove this option and only use the current version in the local or in cluster mode 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see from your flag description
--targeted-k8s-version string Kubernetes version to migrate to (eg 1.24)
The use case for this auditor is for doing Kubernetes version migrations?
Based on the description of the auditor "This command determines which resource is defined with a deprecated API version." I was thinking of "target version" as "check Kubernetes resources against this version", hence it didn't make sense to me to have both "current" and "target" versions. Did you have a specific workflow in mind where you would benefit from passing both current and target versions (eg. running against a live cluster as a one-off before doing a migration)? If so, I think we should document it (eg. as an example) to make the use of the flags more clear. Nothing (other than the target flag description) says anything about migrations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many APIs have been declared deprecated since the 1.16 but are removed from the version 1.22. If the current version is before the 1.16 we can keep the API as it is but if it is not the case, it is better to migrate the API as soon as possible. That is why a warning is generated from the version 1.16.
But as currently kubeaudit
does not retrieve the current version from the cluster we can remove this option in a first time.
dfcd8bd
to
c6067fe
Compare
Description
Fixes #408
Adds a new command
deprecatedapis
to find deprecated APIs of resources. It reports for all resources defined with a deprecated API the version since when it is deprecated the deprecated version, the version where is removed and suggests the recommended API.Here's a sample result:
This PR also brings the possibility to audit any type of resource using the dynamic client.
Type of change
How Has This Been Tested?
Checklist: