Skip to content

Conversation

@oarbusi
Copy link
Collaborator

@oarbusi oarbusi commented Jun 11, 2025

Proposed changes

  • Generate PURLs for all the resources.
  • Adds check in the CI to avoid drift in PURLs when dependencies change
  • Removed serverless-privatelink-endpoint tests in CI (resource is deprecated)
  • Fixed mongoDB version in Online archive tests (was using deprecated version)

Majority of changes in this PR are due to building the resources and generating PURLS. Important changes are in Makefile, /scripts and .github/workflows

Will do a followup PR that will automatically update PURLS in dependabot PRs and SDK Updates

Link to any related issue(s): CLOUDP-324019

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • This change requires a documentation update
  • If changes include removal or addition of 3rd party GitHub actions, I updated our internal document. Reach out to the APIx Integration slack channel to get access to the internal document.

Manual QA performed:

  • cfn invoke for each of CRUDL/cfn test
  • Updated resource in example
  • Published to AWS private registry
  • Used the template in example to create and update a stack in AWS
  • Deleted stack to ensure resources are deleted
  • Created multiple resources in same stack
  • Validated in Atlas UI
  • Included screenshots

Required Checklist:

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that this change does not generate any credentials and that they are NOT accidentally logged anywhere.
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code
  • For CFN Resources: I have released by changes in the private registry and proved by change
    works in Atlas

Further comments

- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36
id: filter
with:
predicate-quantifier: 'every'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This avoids running contract test when only the files in /compliance have changed (see https://github.com/dorny/paths-filter/tree/de90cc6fb38fc0963ad72b210f1f284cd68cea36/ for more details)

@oarbusi oarbusi marked this pull request as ready for review June 11, 2025 15:55
@oarbusi oarbusi requested a review from a team as a code owner June 11, 2025 15:55
run: |
cd cfn-resources
go build -v ./...
- name: Install CloudFormation CLI
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

necessary, as gen-purls runs make build inside the resource directory, and that runs cfn generate

Copy link
Member

Choose a reason for hiding this comment

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

- '!cfn-resources/resource-policy/compliance/**'
search-deployment:
- 'cfn-resources/search-deployment/**'
serverless-private-endpoint:
Copy link
Member

Choose a reason for hiding this comment

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

is serverless-private-endpoint not needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it is deprecated and tests can no longer be run (API does not work). I cleaned this up

_Type_: String

_Minimum_: <code>20</code>
_Minimum Length_: <code>20</code>
Copy link
Member

Choose a reason for hiding this comment

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

it looks like some changes are no related to the main changes of the PR, consider doing them in a different PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This are the changes of building the resources, which is needed to generate the PURLS. We need this if we want to generate the purls

Action:
- "secretsmanager:GetSecretValue"
- "ec2:CreateVpcEndpoint"
- "ec2:DeleteVpcEndpoints"
Copy link
Member

Choose a reason for hiding this comment

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

similar comment, these changes don't seem to be related to the main changes of the PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This are the changes of building the resources, which is needed to generate the PURLS. We need this if we want to generate the purls

Copy link
Member

Choose a reason for hiding this comment

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

can't this built first in a different PR? and then do the pr for purls?

@@ -0,0 +1,19 @@
pkg:golang/../vendor/go.mongodb.org/atlas-sdk/v20231115014@(devel)
Copy link
Member

Choose a reason for hiding this comment

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

if I understand correctly, purls.txt depends on go.mod and go.sum. We share the same go.mod files for all the resources, so all purls.txt will be always be the same.
Do we need to generate all of them, or can we just generate one purls.txt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

purl file is generated from a binary, and a binary is generated for each of the resources, so even if they share go.mod and go.sum, technically the binary is different(and that is what we are shipping) so we have to generate the purls for each

@@ -0,0 +1,30 @@
#!/usr/bin/env bash
Copy link
Member

@lantoli lantoli Jun 12, 2025

Choose a reason for hiding this comment

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

consider exposing check & generate purls GHAs from TF repo so all the repos can use it, and we don't need the scripts & make goals in all the repos

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

CFN and TF are built different, so it's not exactly the same and wouldn't be 100% reusable. For now will keep it as is but will keep an eye for next repos.

Copy link
Member

Choose a reason for hiding this comment

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

ok, that's fair

@oarbusi oarbusi closed this Jun 12, 2025
@oarbusi
Copy link
Collaborator Author

oarbusi commented Jun 12, 2025

Will not put PURLS in the repo anymore, and will generate it only when needed for the release

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.

3 participants