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

api_server_encryption_provider_cipher rule.yml has bad jsonpath #11099

Merged
merged 2 commits into from
Mar 12, 2024

Conversation

prb112
Copy link
Contributor

@prb112 prb112 commented Sep 8, 2023

Description:

  • api_server_encryption_provider_cipher rule.yml has bad jsonpath

Rationale:

  • The jsonpath returns nothing, with the update, it processes the write results.

Review Hints:

as-is

❯ oc get openshiftapiserver -o=jsonpath='{range.items[0].status.conditions[?(@.type=="Encrypted")]}{.reason}{"\n"}{.message}{"\n"}'

with fix:

❯ oc get openshiftapiserver -o=jsonpath='{range .items[0]}{.status.conditions[?(@.type=="Encrypted")].status}{"\n"}{end}'
False

@rhmdnd This might be you or @Vincent056 to review.

api_server_encryption_provider_cipher rule.yml has bad jsonpath

Signed-off-by: Paul Bastide <pbastide@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Sep 8, 2023

Hi @prb112. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label Sep 8, 2023
@github-actions
Copy link

github-actions bot commented Sep 8, 2023

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@Mab879 Mab879 added the OpenShift OpenShift product related. label Sep 8, 2023
@yuumasato yuumasato self-assigned this Sep 27, 2023
@@ -76,7 +76,7 @@ ocil: |-
API server to verify that its resources were successfully encrypted:
<pre>
# encrypt the etcd datastore
$ oc get openshiftapiserver -o=jsonpath='{range.items[0].status.conditions[?(@.type=="Encrypted")]}{.reason}{"\n"}{.message}{"\n"}'
$ oc get openshiftapiserver -o=jsonpath='{range .items[0]}{.status.conditions[?(@.type=="Encrypted")].status}{"\n"}{end}'
Copy link
Member

Choose a reason for hiding this comment

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

@prb112 Hi,

The jsonpath itself is not wrong, but it doesn't show anything meaningful when etcd encryption is not configured.

Soon after encryption is configured it outputs the following:

oc get openshiftapiserver -o=jsonpath='{range .items[0].status.conditions[?(@.type=="Encrypted")]}{.reason}{"\n"}{.message}{"\n"}'                                                                                                                                    
EncryptionInProgress
Resource routes.route.openshift.io is not encrypted


And when it is completed:

oc get openshiftapiserver -o=jsonpath='{range .items[0].status.conditions[?(@.type=="Encrypted")]}{.reason}{"\n"}{.message}{"\n"}'
EncryptionCompleted
All resources encrypted: routes.route.openshift.io


How about adding .status to the original jsonpath?

Suggested change
$ oc get openshiftapiserver -o=jsonpath='{range .items[0]}{.status.conditions[?(@.type=="Encrypted")].status}{"\n"}{end}'
$ oc get openshiftapiserver -o=jsonpath='{range .items[0].status.conditions[?(@.type=="Encrypted")]}{.status}{"\n"}{.reason}{"\n"}{.message}{"\n"}{end}

Copy link
Member

Choose a reason for hiding this comment

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

I made the change on a4c0420

@vojtapolasek vojtapolasek added this to the 0.1.72 milestone Nov 29, 2023
Modify oc jsonpath to more clearly show the status of etcd's
encryption provider.
This is relevant when no encryption is configured and reason and message
are empty.
Copy link

codeclimate bot commented Dec 5, 2023

Code Climate has analyzed commit a4c0420 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 58.5%.

View more on Code Climate.

@BhargaviGudi
Copy link
Collaborator

/hold for test

@openshift-ci openshift-ci bot added the do-not-merge/hold Used by openshift-ci-robot bot. label Dec 28, 2023
@BhargaviGudi
Copy link
Collaborator

Verification passed with 4.14.7 + compliance-operator from ComplianceAsCode

$ oc get rules upstream-ocp4-api-server-encryption-provider-cipher -ojsonpath={.instructions}
OpenShift supports encryption of data at rest of etcd datastore, but it is up to the
customer to configure. The asecbc cipher is used. No other ciphers are supported. Keys
are stored on the filesystem of the master and automatically rotated.
Run the following command to review the Encrypted status condition for the OpenShift
API server to verify that its resources were successfully encrypted:

# encrypt the etcd datastore
$ oc get openshiftapiserver -o=jsonpath='{range .items[0].status.conditions[?(@.type=="Encrypted")]}{.status}{"\n"}{.reason}{"\n"}{.message}{"\n"}{end}'

The output shows EncryptionCompleted upon successful encryption.
If the output shows EncryptionInProgress this means that encryption is still in
progress. Wait a few minutes and try again.
To display the encryption configured, run the following command:
$ oc get --raw /apis/config.openshift.io/v1/apiservers/cluster | jq [.spec.encryption.type] 
If the output does not list aescbc, the encryption is not configured correctly.
$ oc get openshiftapiserver -o=jsonpath='{range .items[0].status.conditions[?(@.type=="Encrypted")]}{.status}{"\n"}{.reason}{"\n"}{.message}{"\n"}{end}'
False


$ oc get --raw /apis/config.openshift.io/v1/apiservers/cluster | jq [.spec.encryption.type] 
[
  null
]

@BhargaviGudi
Copy link
Collaborator

/unhold

@openshift-ci openshift-ci bot removed the do-not-merge/hold Used by openshift-ci-robot bot. label Dec 28, 2023
@BhargaviGudi
Copy link
Collaborator

label /qe-approved

@marcusburghardt marcusburghardt modified the milestones: 0.1.72, 0.1.73 Jan 29, 2024
@yuumasato
Copy link
Member

Thanks @rhmdnd .
I re-run the failing testing-farm test and this time it passed.
Merging.

@yuumasato yuumasato merged commit f73d37e into ComplianceAsCode:master Mar 12, 2024
37 checks passed
@Mab879 Mab879 added the Update Rule Issues or pull requests related to Rules updates. label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ok-to-test Used by openshift-ci bot. OpenShift OpenShift product related. Update Rule Issues or pull requests related to Rules updates.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants