Skip to content

Conversation

@yelizhenden-mdb
Copy link
Collaborator

@yelizhenden-mdb yelizhenden-mdb commented Apr 1, 2025

Proposed changes

Jira ticket: CLOUDP-306576

  xgen-IPA-119-no-default-for-cloud-providers:
    description: |
      When using a provider field or parameter, API producers should not define a default value.
      This rule checks fields and parameters named "cloudProvider" and ensures they do not have a default value.
      It also checks enum fields that might contain cloud provider values.
      All cloudProviderEnumValues should be listed in the enum array.

Found 8 violations, all of them input parameters

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works

Changes to Spectral

  • I have read the README file for Spectral Updates

Further comments

@wtrocki wtrocki requested a review from Copilot April 1, 2025 19:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new Spectral rule (IPA-119) to enforce that API producers do not set default values for cloud provider fields or parameters, thereby supporting multi-cloud by default.

  • Implements the rule function in IPA119NoDefaultForCloudProviders.js
  • Updates rule definitions in IPA-119.yaml and documentation in README.md
  • Adds comprehensive tests in IPA119NoDefaultForCloudProviders.test.js and updates the overall rule suite in ipa-spectral.yaml

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tools/spectral/ipa/rulesets/functions/IPA119NoDefaultForCloudProviders.js Implements the logic for detecting default values on provider fields or parameters with exception handling.
tools/spectral/ipa/rulesets/README.md Documents the new rule and its usage.
tools/spectral/ipa/rulesets/IPA-119.yaml Provides the rule metadata and configuration.
tools/spectral/ipa/ipa-spectral.yaml Updates the rule suite to include the new IPA-119 rule.
tools/spectral/ipa/tests/IPA119NoDefaultForCloudProviders.test.js Contains tests validating the new rule’s behavior.
Comments suppressed due to low confidence (1)

tools/spectral/ipa/rulesets/functions/IPA119NoDefaultForCloudProviders.js:41

  • Consider checking whether propertyObject.schema exists before accessing its 'default' property to avoid potential runtime exceptions when the schema is missing.
if (propertyObject.name === propertyNameToLookFor && propertyObject.schema.default !== undefined) {

@yelizhenden-mdb yelizhenden-mdb marked this pull request as ready for review April 2, 2025 10:05
@yelizhenden-mdb yelizhenden-mdb requested a review from a team as a code owner April 2, 2025 10:05
Copy link
Member

@wtrocki wtrocki left a comment

Choose a reason for hiding this comment

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

LGTM - on behalf of copilot ;)

@yelizhenden-mdb yelizhenden-mdb merged commit cdc3e8a into main Apr 2, 2025
8 checks passed
@yelizhenden-mdb yelizhenden-mdb deleted the CLOUDP-306576 branch April 2, 2025 10:14
severity: warn
message: '{{error}} https://mdb.link/mongodb-atlas-openapi-validation#xgen-IPA-119-no-default-for-cloud-providers'
given:
# Target properties with "cloudProvider" in their name
Copy link
Collaborator

Choose a reason for hiding this comment

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

I saw some examples of providerName is the spec as well, perhaps these can be included as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the enum check will capture the providerName parameters. For example: paths//api/atlas/v2/groups/{groupId}/containers/get/parameters/6

) {
try {
if (fieldType === 'properties') {
if (propertyName === propertyNameToLookFor && propertyObject.default !== undefined) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q: If there are no properties with the name cloudProvider, will this collect an adoption? Perhaps we should filter these out before the collectAndReturnViolation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that's a good point! Let me open a PR for it

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.

4 participants