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 smoke test credential usage #88

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

rick-a-lane-ii
Copy link
Collaborator

@rick-a-lane-ii rick-a-lane-ii commented Mar 13, 2024

Description

Mimics the change made to matrix.yaml recently. This ensures that both Kubeception credentials and GKE credentials are not simultaneously required for every cluster. This was a previous defect (that has now been fixed in #85) that was never caught due to the way these tests are structured.

It's a slightly cheesy way of accomplishing this, but it's better than a more substantial rework of the testing.

Blast Radius

None.

Dependencies and documentation

Documentation

  • I have updated user facing documentation.
  • I have updated all Runbooks affected by this change.
  • I updated DEVELOPING.md with any dev tricks I used to work on this code efficiently.
  • My changes do not have any impact on documentation.

Monitoring

  • I have verified that any changes to alerts work as expected.
  • My changes affect monitoring rules and/or dashboards, and I made sure they didn't break.
  • My changes do not have any impact on monitoring.

External dependencies

  • I have validated that dependencies impacted by this change work as expected.
  • My changes do not impact external dependencies.

Rosie the Robot checklists

  • My changes affect one or more Rosie checklists.
  • My changes do not have any impact on Rosie's checklists.

Testing

  • I have validated that my changes work as expected.
  • My changes do not require testing.

Testing Strategy

Existing tests.

Security

  • The changes in this PR have been reviewed for security concerns and adherence to security best practices.

Related Issues or corrective actions

N/A

Deployment plan

Ensure test runs successfully after merge.

@rick-a-lane-ii rick-a-lane-ii requested a review from a team March 13, 2024 21:35
@@ -35,8 +38,8 @@ jobs:
distribution: ${{ matrix.clusters.distribution }}
version: ${{ matrix.clusters.version }}
kubeconfig: ${{ runner.temp}}/kubeconfig.yaml
kubeceptionToken: ${{ secrets.KUBECEPTION_TOKEN }}
gkeCredentials: ${{ secrets.GOOGLE_APPLICATION_CREDENTIALS }}
kubeceptionToken: ${{ matrix.clusters.distribution == 'Kubeception' && env.KUBECEPTION_TOKEN }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer that the parameter was not set, in case there are issues handling missing parameters, but I think this works for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I just didn't want to redo all of these right now.
Ideally these are split apart with different sets of parameters, but it would take a bit of work to change/test these.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#89

@rick-a-lane-ii rick-a-lane-ii merged commit 2193a79 into main Mar 14, 2024
9 checks passed
@rick-a-lane-ii rick-a-lane-ii deleted the rlane/update-smoke-test-auth branch March 14, 2024 14:51
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.

2 participants