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

[Security Hardened Shoot Cluster] Rule 1002 implementation #433

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

AleksandarSavchev
Copy link
Member

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #430

Special notes for your reviewer:

Release note:

Implementation for rule `1002` from the `security-hardened-shoot-cluster` ruleset for provider `garden`.

@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 7, 2025
@gardener-robot gardener-robot added the needs/review Needs review label Feb 7, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 7, 2025
} else if kind == gardencorev1beta1constants.CloudProfileReferenceKindNamespacedCloudProfile {
namespacedCloudProfile = &gardencorev1beta1.NamespacedCloudProfile{ObjectMeta: v1.ObjectMeta{Name: cloudProfileName, Namespace: r.ShootNamespace}}
if err := r.Client.Get(ctx, client.ObjectKeyFromObject(namespacedCloudProfile), namespacedCloudProfile); err != nil {
return rule.Result(r, rule.ErroredCheckResult(err.Error(), rule.NewTarget("name", cloudProfileName, "namespace", r.ShootNamespace, "kind", "NamespacedCloudProfile"))), nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return rule.Result(r, rule.ErroredCheckResult(err.Error(), rule.NewTarget("name", cloudProfileName, "namespace", r.ShootNamespace, "kind", "NamespacedCloudProfile"))), nil
return rule.Result(r, rule.ErroredCheckResult(err.Error(), rule.NewTarget("name", cloudProfileName, "namespace", r.ShootNamespace, "kind", gardencorev1beta1constants.CloudProfileReferenceKindNamespacedCloudProfile))), nil

}
}

return []string{string(core.ClassificationSupported)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we directly use the Classification type?

Comment on lines 172 to 178
if version.Classification == nil {
return rule.FailedCheckResult("Worker group uses image with unclassified image.", target), true
} else if slices.Contains(r.acceptedClassifications(imageName), string(*version.Classification)) {
return rule.PassedCheckResult("Worker group has accepted image.", target.With("classification", string(*version.Classification))), true
} else {
return rule.FailedCheckResult("Worker group has not accepted image.", target.With("classification", string(*version.Classification))), true
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use a switch

return rule.ErroredCheckResult("image version not found in namespacedCloudProfile", target)
}

func (r *Rule1002) checkCloudProfile(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not needed if only used in one place

return rule.Result(r, checkResults...), nil
}

func (r *Rule1002) checkImageVersion(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function can be removed?

# - name: image-name
# expectedClassifications:
# - supported
# - supported
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate rows. Probably a typo

@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Feb 10, 2025
@gardener-robot gardener-robot added size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Feb 10, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 10, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 10, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 10, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 10, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 10, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 10, 2025
@AleksandarSavchev AleksandarSavchev marked this pull request as ready for review February 10, 2025 14:32
@AleksandarSavchev AleksandarSavchev requested a review from a team as a code owner February 10, 2025 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/changes Needs (more) changes needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/review Needs review needs/second-opinion Needs second review by someone else size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security Hardened Shoot Cluster] Add rule 1002 to check worker's image versions
5 participants