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.

Sign up for GitHub

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

Jump to bottom

[skunkworks] Auditing contract testing #1589

Merged
merged 4 commits into from
Jun 4, 2024
Merged

[skunkworks] Auditing contract testing #1589

merged 4 commits into from
Jun 4, 2024

Conversation

josvazg
Copy link
Collaborator

@josvazg josvazg commented May 14, 2024 •
edited
Loading

These is the sample "foundations" PR of a new feature. It does NOT add anything that cannot be merged ASAP. Will hold it as it is a PoC.

It includes:

  • Auditing translation layer at internal/translayer/auditing.
  • Contract tests for the translation layer at test/contract/auditing.

Screenshot 2024-05-15 at 09 03 46

Infrastructure that had to be added as a one off, if this approach is followed, next PRs like will not need it:

  • Contract tests workflow and wiring from cloud tests:
    • .github/workflows/contract-tests.yml & .github/workflows/cloud-tests.yml
  • Added a rule for contract tests to the Makefile.
  • Client building function for translation layer services at internal/translater/client.go.
  • Launcher infrastructure for contract tests at test/helper/launcher.
  • A minor improvement for skipping tests at test/helper/control/enable.go.
    • And its new usages.

Engineering Proposal: Translation Layer

Also related to CLOUDP-242997

All Submissions:

  • Have you signed our CLA?

@josvazg josvazg marked this pull request as draft May 14, 2024 15:53
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from 97efc28 to 71890fd Compare May 14, 2024 16:22
@josvazg josvazg added the cloud-tests Run expensive Cloud Tests: Integration & E2E label May 14, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 14, 2024 •
edited
Loading

https://app.codecov.io/github/mongodb/mongodb-atlas-kubernetes/commit/d94928b66c9fb41bd2731a99c2768c4bdb0797ec

@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from 71890fd to 80613e3 Compare May 14, 2024 16:40
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from 80613e3 to 36d5fd2 Compare May 14, 2024 16:51
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from 36d5fd2 to d3df81c Compare May 14, 2024 16:59
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch 2 times, most recently from 439dd3a to 0629999 Compare May 14, 2024 17:52
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from 0629999 to 6112b30 Compare May 14, 2024 19:30
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from 6112b30 to 58a3d37 Compare May 17, 2024 12:18
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from 58a3d37 to a45a263 Compare May 20, 2024 07:42
@josvazg josvazg removed the cloud-tests Run expensive Cloud Tests: Integration & E2E label May 20, 2024
@josvazg josvazg marked this pull request as ready for review May 20, 2024 08:00
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from a08fa0f to ca364fd Compare May 22, 2024 14:39
helderjs
helderjs requested changes May 30, 2024
View reviewed changes
Copy link
Collaborator

@helderjs helderjs left a comment

Choose a reason for hiding this comment

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

The setup of the env, using Kind and the operator itself to prepare things for the test doesn't make me happy. Don't you think that using the Atlas client directly is a better idea? Like this, the contract test would have zero dependencies to be run.

internal/translayer/auditing/translate.go Outdated Show resolved Hide resolved
internal/translayer/auditing/translate.go Outdated Show resolved Hide resolved
internal/translayer/client.go Outdated Show resolved Hide resolved
@josvazg
Copy link
Collaborator Author

josvazg commented May 31, 2024

The setup of the env, using Kind and the operator itself to prepare things for the test doesn't make me happy. Don't you think that using the Atlas client directly is a better idea? Like this, the contract test would have zero dependencies to be run.

This way we are dogfooding, we depend on us.

I did a try without the operator and it got quite complex. Note we need not only provide a project, or a project and a deployment, but arbitrary setups, which the operator can do just by passing it a YAML input file.

I think my old attempt is there closed somewhere.

@josvazg
Copy link
Collaborator Author

josvazg commented May 31, 2024

@helderjs I found my old attempt, see that is ugly as hell: #1438

Do you have something else in mind?

@josvazg josvazg force-pushed the skunkworks/auditing-contract branch 2 times, most recently from bef3485 to 0bf1a2e Compare June 3, 2024 11:47
s-urbaniak
s-urbaniak requested changes Jun 4, 2024
View reviewed changes
Copy link
Collaborator

@s-urbaniak s-urbaniak left a comment

Choose a reason for hiding this comment

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

Looks like a great direction, a couple of comments and concerns 👍

internal/translation/auditing/auditing.go Outdated Show resolved Hide resolved
internal/translation/auditing/auditing.go Outdated Show resolved Hide resolved
internal/translation/auditing/auditing.go Outdated Show resolved Hide resolved
internal/translation/auditing/conversion.go Outdated Show resolved Hide resolved
internal/translation/auditing/conversion.go Outdated Show resolved Hide resolved
test/helper/launcher/launcher.go Outdated Show resolved Hide resolved
test/helper/launcher/launcher.go Outdated Show resolved Hide resolved
test/helper/launcher/launcher.go Show resolved Hide resolved
test/helper/launcher/launcher_test.go Outdated Show resolved Hide resolved
test/helper/launcher/wait.go Show resolved Hide resolved
@josvazg
[skunkworks] Auditing contract testing
81c2904 
Signed-off-by: jose.vazquez <jose.vazquez@mongodb.com>
@josvazg
Rename & refactor service
c43879e
@josvazg
Renames for consistency
de8fccb 
Signed-off-by: jose.vazquez <jose.vazquez@mongodb.com>
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from 0bf1a2e to 2f214d8 Compare June 4, 2024 12:19
@josvazg josvazg requested a review from s-urbaniak June 4, 2024 12:33
@josvazg
Copy link
Collaborator Author

josvazg commented Jun 4, 2024

@s-urbaniak I believe I applied all your feedback.

@josvazg
Applied Sergiusz suggestions
d94928b 
Signed-off-by: jose.vazquez <jose.vazquez@mongodb.com>
@josvazg josvazg force-pushed the skunkworks/auditing-contract branch from 2f214d8 to d94928b Compare June 4, 2024 12:56
@josvazg josvazg merged commit b8a2adf into main Jun 4, 2024
56 checks passed
@josvazg
Copy link
Collaborator Author

josvazg commented Jun 4, 2024

Related to CLOUDP-242997

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@helderjs helderjs helderjs approved these changes

@s-urbaniak s-urbaniak s-urbaniak approved these changes

@roothorp roothorp roothorp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@josvazg @helderjs @s-urbaniak @roothorp