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

Update TestBpfmanConfigReconcileAndDelete unit test to verify that the OpenShift SCC is deployed #65

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

frobware
Copy link
Contributor

Fixes #27.

This pull request refactors the existing TestBpfmanConfigReconcileAndDelete function to verify that the OpenShift Security Context Constraints (SCC) resource is created and deleted during reconciliation. The previous implementation did not test the OpenShift case.

Summary

  • Introduce setupTestEnvironment helper function.
  • Refactor TestBpfmanConfigReconcileAndDelete to use a table-driven approach.
  • Verify creation and configuration of Restricted SCC in OpenShift.
  • Update reconciler function to ensure proper cleanup of Restricted SCC in OpenShift case.

These are the key changes:

  1. Introduce setupTestEnvironment Helper Function:

    • Standardise the setup process for TestBpfmanConfigReconcileAndDelete tests.
    • Handle the initialisation of the fake client, the registration of necessary types with the runtime scheme, and the configuration of the reconciler.
    • Enable specific OpenShift testing by conditionally setting the RestrictedSCC field on the reconciler object.
    • Eliminate duplicated path references to various YAML files by using constants declared in the internal package and resolving the paths relative to the test directory at runtime. This ensures that if filenames change, the test code doesn't need to be updated.
  2. Refactor TestBpfmanConfigReconcileAndDelete:

    • Convert the test to a table-driven approach, enabling testing for both standard and OpenShift scenarios.
    • Verify that the Restricted Security Context Constraints (SCC) is created and configured correctly when the isOpenShift flag is true.
    • Identify that the Restricted SCC was not being deleted during the reconciliation process. Update the reconciler function to address this omission, ensuring proper cleanup of resources in the OpenShift case.
    • Use the new setupTestEnvironment helper function to standardise the setup process.

frobware added 2 commits July 24, 2024 17:44
Introduce a new test helper function `setupTestEnvironment` to
standardise the setup process for BpfmanConfigReconciler tests. This
function handles the initialisation of the fake client, the registration
of necessary types with the runtime scheme, and the configuration of the
reconciler.

The helper enables specific OpenShift testing by conditionally setting
the RestrictedSCC field on the reconciler object.

The logic is taken from the existing TestBpfmanConfigReconcileAndDelete
function. The previous setup logic duplicated paths to various YAML
files by pre-pending ../../. This new setup function dispenses with the
duplicated paths by using the constants declared in the internal package
and resolves the path at runtime to the actual files in the filesystem
relative to the test directory. This allows us to refer to the canonical
path without having to duplicate it, ensuring that if we ever change the
filename then the test code doesn't need to be updated.

This commit allows us to share the test setup logic so that we can
subsequently modify TestBpfmanConfigReconcileAndDelete to verify that
the Restricted SCC is created in the OpenShift test case, which was
previously not tested.

Signed-off-by: Andrew McDermott <[email protected]>
…Shift

Refactor the TestBpfmanConfigReconcileAndDelete function to use a
table-driven approach, enabling testing for both standard and OpenShift
environments. This change introduces testing for OpenShift by verifying
that the Restricted Security Context Constraints (SCC) is created and
configured correctly when the isOpenShift flag is true.

Additionally, this test identified that the Restricted SCC was not being
deleted during the reconciliation process. The reconciler function has
been updated to address this omission, ensuring proper cleanup of
resources in the OpenShift case.

The refactored test uses the new setupTestEnvironment helper function to
standardise the setup process.

Signed-off-by: Andrew McDermott <[email protected]>
@frobware frobware force-pushed the issue-27/test/openshift-scc-deployment branch from 4f27d9f to 375cbf2 Compare July 24, 2024 16:44
Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.

Project coverage is 27.66%. Comparing base (bf7994e) to head (375cbf2).
Report is 18 commits behind head on main.

Files Patch % Lines
controllers/bpfman-operator/configmap.go 63.63% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #65      +/-   ##
==========================================
+ Coverage   27.39%   27.66%   +0.27%     
==========================================
  Files          81       81              
  Lines        6968     6986      +18     
==========================================
+ Hits         1909     1933      +24     
+ Misses       4873     4864       -9     
- Partials      186      189       +3     
Flag Coverage Δ
unittests 27.66% <63.63%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -173,6 +173,20 @@ func (r *BpfmanConfigReconciler) ReconcileBpfmanConfig(ctx context.Context, req
return ctrl.Result{Requeue: true, RequeueAfter: retryDurationOperator}, nil
}

if r.IsOpenshift {
Copy link
Contributor

Choose a reason for hiding this comment

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

good catch!!

@msherif1234
Copy link
Contributor

/LGTM Thanks @frobware

@mergify mergify bot merged commit 18cb81a into bpfman:main Jul 31, 2024
16 checks passed
msherif1234 pushed a commit to msherif1234/bpfman-operator that referenced this pull request Nov 6, 2024
…/ocp-bpfman-agent

chore(deps): update ocp-bpfman-agent to 1211a10
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

Successfully merging this pull request may close these issues.

Write a unit test for deploying the Openshift SCC
2 participants