Skip to content

Fix infra secret lookup so that it defaults to the namespace of the referencing resource#167

Merged
k8s-ci-robot merged 2 commits into
kubernetes-sigs:mainfrom
pjaton:fix-infra-secret-lookup
Aug 1, 2022
Merged

Fix infra secret lookup so that it defaults to the namespace of the referencing resource#167
k8s-ci-robot merged 2 commits into
kubernetes-sigs:mainfrom
pjaton:fix-infra-secret-lookup

Conversation

@pjaton
Copy link
Copy Markdown
Contributor

@pjaton pjaton commented Aug 1, 2022

What this PR does / why we need it:

This change fixes where the GenerateInfraClusterClient function looks up for the infrastructure secret referenced in a KubevirtCluster or KubevirtMachine, when no namespace is specified in the ObjectReference.

This corrects the current logic by ensuring that, in this case, the code looks for the secret in the same namespace than the referencing resource; which aligns with the default behavior that CAPI follows for other ObjectReferences.

To validate this change of behavior, this PR also introduce a set of tests for the infracluster package.

Which issue this PR fixes:

fixes #165

Special notes for your reviewer:

Note that this change is split in two commits:

  • The first one with a refactoring that extract the duplicated setupScheme() function into as single function in the testing package.
  • The second commit introduces the fix and the tests validating the infracluster package.

Release notes:

NONE

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 1, 2022
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 1, 2022
@pjaton pjaton changed the title Fix infra secret lookup Fix infra secret lookup so that it defaults to the namespace of the referencing resource Aug 1, 2022
@coveralls
Copy link
Copy Markdown

coveralls commented Aug 1, 2022

Pull Request Test Coverage Report for Build 2775158040

  • 14 of 14 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.1%) to 49.486%

Totals Coverage Status
Change from base Build 2763291491: 1.1%
Covered Lines: 819
Relevant Lines: 1655

💛 - Coveralls

Also, add unit-test for the infracluster package.
@davidvossel
Copy link
Copy Markdown
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Aug 1, 2022
Copy link
Copy Markdown
Contributor

@davidvossel davidvossel left a comment

Choose a reason for hiding this comment

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

/approve

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidvossel, pjaton

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 1, 2022
@agradouski
Copy link
Copy Markdown
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 1, 2022
@k8s-ci-robot k8s-ci-robot merged commit 5aa3a3d into kubernetes-sigs:main Aug 1, 2022
@pjaton pjaton deleted the fix-infra-secret-lookup branch August 1, 2022 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CAPK should look for infraClusterSecret in the KubevirtCluster resource namespace by default

5 participants