Skip to content

Add stable version for Connectordefinitions API#30812

Merged
xuhumsft merged 5 commits intoAzure:dev-Sentinel-securityinsights-Microsoft.SecurityInsights-2024-09-01from
sagamzu:AddStableVersionForConnectorDefinitionsApi
Nov 6, 2024
Merged

Add stable version for Connectordefinitions API#30812
xuhumsft merged 5 commits intoAzure:dev-Sentinel-securityinsights-Microsoft.SecurityInsights-2024-09-01from
sagamzu:AddStableVersionForConnectorDefinitionsApi

Conversation

@sagamzu
Copy link
Contributor

@sagamzu sagamzu commented Oct 1, 2024

ARM (Control Plane) API Specification Update Pull Request

Tip

Overwhelmed by all this guidance? See the Getting help section at the bottom of this PR description.

PR review workflow diagram

Please understand this diagram before proceeding. It explains how to get your PR approved & merged.

spec_pr_review_workflow_diagram

Purpose of this PR

What's the purpose of this PR? Check the specific option that applies. This is mandatory!

  • New resource provider.
  • New API version for an existing resource provider. (If API spec is not defined in TypeSpec, the PR should have been created in adherence to OpenAPI specs PR creation guidance).
  • Update existing version for a new feature. (This is applicable only when you are revising a private preview API version.)
  • Update existing version to fix OpenAPI spec quality issues in S360.
  • Convert existing OpenAPI spec to TypeSpec spec (do not combine this with implementing changes for a new API version).
  • Other, please clarify:
    • edit this with your clarification

Due diligence checklist

To merge this PR, you must go through the following checklist and confirm you understood
and followed the instructions by checking all the boxes:

  • I confirm this PR is modifying Azure Resource Manager (ARM) related specifications, and not data plane related specifications.
  • I have reviewed following Resource Provider guidelines, including
    ARM resource provider contract and
    REST guidelines (estimated time: 4 hours).
    I understand this is required before I can proceed to the diagram Step 2, "ARM API changes review", for this PR.

Additional information

Viewing API changes

For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.

Suppressing failures

If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the
suppressions guide to get approval.

Getting help

  • First, please carefully read through this PR description, from top to bottom. Please fill out the Purpose of this PR and Due diligence checklist.
  • If you don't have permissions to remove or add labels to the PR, request write access per aka.ms/azsdk/access#request-access-to-rest-api-or-sdk-repositories
  • To understand what you must do next to merge this PR, see the Next Steps to Merge comment. It will appear within few minutes of submitting this PR and will continue to be up-to-date with current PR state.
  • For guidance on fixing this PR CI check failures, see the hyperlinks provided in given failure
    and https://aka.ms/ci-fix.
  • For help with ARM review (PR workflow diagram Step 2), see https://aka.ms/azsdk/pr-arm-review.
  • If the PR CI checks appear to be stuck in queued state, please add a comment with contents /azp run.
    This should result in a new comment denoting a PR validation pipeline has started and the checks should be updated after few minutes.
  • If the help provided by the previous points is not enough, post to https://aka.ms/azsdk/support/specreview-channel and link to this PR.

Add stable version for Connectordefinitions API.
Please note that this is exactly the same doc as we have for the lates preview version

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Oct 1, 2024

Next Steps to Merge

Next steps that must be taken to merge this PR:
  • ❌ Your PR has breaking changes in the generated SDK for Go (label: BreakingChange-Go-Sdk). Refer to step 3 in the PR workflow diagram.

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Oct 1, 2024

@sagamzu sagamzu marked this pull request as ready for review October 1, 2024 09:36
@stavbella stavbella force-pushed the AddStableVersionForConnectorDefinitionsApi branch from 00d9f56 to ac6b8a6 Compare October 15, 2024 07:09
@stavbella stavbella force-pushed the AddStableVersionForConnectorDefinitionsApi branch from ac6b8a6 to 80aeca7 Compare October 15, 2024 07:12
@xuhumsft xuhumsft added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Oct 15, 2024
"status": {
"$ref": "#/definitions/AvailabilityStatus"
},
"isPreview": {
Copy link
Member

Choose a reason for hiding this comment

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

isPreview

ARM recommends enum instead of boolean. Consider something like stage: { Preview, Stable }. You can add future values later, e.g. Private, Deprecated, Removed, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mentat9
The reason we didnt use Enum, was to save backword compatability for the customers, since thery are using today the same object in another API. This was the Product Manager desicion.

This API was review in preview version.
we want to have exactly the same api in Stable version.

"description": "The exposure status of the connector to the customers.",
"type": "object",
"properties": {
"status": {
Copy link
Member

Choose a reason for hiding this comment

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

status

Why not use string enum, and combine it with isPreview boolean, e.g.
availability: {
Disabled (==None),
PrivatePreview (==FeatureFlag+isPreview)
Private (==FeatureFLag),
Preview (==Available+isPreview),
Public (==Available),
Internal
}
. . . or similar names you like better.

This approach is easier for clients to understand/use, and designs out wrong combinations (like None + isPreview), but still allows easily adding new availability values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mentat9 same as previous comment

],
"type": "object",
"properties": {
"name": {
Copy link
Member

Choose a reason for hiding this comment

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

name

Don't use top-level name in the property bag. Could use connectorType instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mentat9 same as previous comment

"description": "An Azure resource, which encapsulate the entire info requires to display a data connector page in Azure portal,\r\nand the info required to define data connections.",
"type": "object",
"properties": {
"kind": {
Copy link
Member

Choose a reason for hiding this comment

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

kind

Tracked resources use "kind" as a top-level property name. Don't use it within the property bag. Maybe connectorKind is suitable.

"connectorUiConfig"
],
"properties": {
"createdTimeUtc": {
Copy link
Member

Choose a reason for hiding this comment

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

createdTimeUtc

Use the top-level systemData properties instead of duplicating it in the property bag.

"description": "Gets or sets the connector definition created date in UTC format.",
"type": "string"
},
"lastModifiedUtc": {
Copy link
Member

Choose a reason for hiding this comment

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

lastModifiedUtc

Use property in top-level systemData instead.

],
"type": "object",
"properties": {
"id": {
Copy link
Member

Choose a reason for hiding this comment

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

id

Don't duplicate top-level ARM properties. If this is an Azure resourceId, you should use the name resourceId or connectorResourceId, and add format: arm-id. if it's something else, clarify what it is in the description (e.g. number, name, guid, etc.).

"description": "Gets or sets custom connector id. optional field.",
"type": "string"
},
"title": {
Copy link
Member

Choose a reason for hiding this comment

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

title

This kind of property is usually called displayName.

},
"logo": {
"description": "Gets or sets the connector logo to be used when displaying the connector within Azure Sentinel's connector's gallery.\r\nThe logo value should be in SVG format.",
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

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

"type": "string"

Description should also explain how this property is encoded into a string.

"description": "Gets or sets the connector logo to be used when displaying the connector within Azure Sentinel's connector's gallery.\r\nThe logo value should be in SVG format.",
"type": "string"
},
"isConnectivityCriteriasMatchSome": {
Copy link
Member

Choose a reason for hiding this comment

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

isConnectivityCriteriasMatchSome

Recommend enum instead of boolean, e.g. criteriaCondition: { And, Or }

],
"type": "object",
"properties": {
"name": {
Copy link
Member

Choose a reason for hiding this comment

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

name

Duplicates top-level ARM property name, could be permissionName.

"description": "Instruction steps to enable the connector.",
"type": "object",
"properties": {
"title": {
Copy link
Member

Choose a reason for hiding this comment

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

title

Consider using displayName (not a requirement).

},
"x-ms-identifiers": []
},
"innerSteps": {
Copy link
Member

Choose a reason for hiding this comment

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

innerSteps

Hard to see how this is different/better than just putting everything into one "instructions" list. How does a client combine the two lists?

"description": "Gets or sets the instruction type parameters settings.",
"type": "object"
},
"type": {
Copy link
Member

Choose a reason for hiding this comment

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

type

Don't duplicate top-level property name. Could use instructionType.

"description": "Gets or sets the required tenant permissions for the connector.",
"type": "array",
"items": {
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

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

"type": "string"

What format? At minimum include format information in the description.

"description": "Gets or sets the required licenses for the user to create connections.",
"type": "array",
"items": {
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

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

"type": "string"

What string format does a "license" have? Beef up the description so a client knows what these should be.

"properties": {
"provider": {
"description": "Gets or sets the provider name.",
"type": "string"
Copy link
Member

Choose a reason for hiding this comment

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

"type": "string"

Format? Is this like an RP namespace name?

@openapi-pipeline-app openapi-pipeline-app bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Oct 15, 2024
@sagamzu
Copy link
Contributor Author

sagamzu commented Nov 5, 2024

Data connector definition API reviewed in Preview version.
We fixed all the comments we got on the version.

The customers are using this API for a while, the feature is in GA for a few month and we want to have exactly the same API in Stable version.

Please see the Preview PR:
First PR: #27736
Second PR: https://github.com/Azure/azure-rest-api-specs/pull/27737/files

and you can see the merged code here:
https://github.com/Azure/azure-rest-api-specs/blob/main/specification/securityinsights/resource-manager/Microsoft.SecurityInsights/preview/2024-04-01-preview/dataConnectorDefinitions.json

@xuhumsft xuhumsft added WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required and removed ARMChangesRequested labels Nov 5, 2024
@raosuhas raosuhas added the ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review label Nov 5, 2024
@openapi-pipeline-app openapi-pipeline-app bot removed the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Nov 5, 2024
@xuhumsft xuhumsft merged commit c632444 into Azure:dev-Sentinel-securityinsights-Microsoft.SecurityInsights-2024-09-01 Nov 6, 2024
xuhumsft added a commit that referenced this pull request Nov 20, 2024
…30800)

* Copy files from stable/2024-03-01

Copied the files in a separate commit.
This allows reviewers to easily diff subsequent changes against the previous spec.

* Update version to stable/2024-09-01

Updated the API version from stable/2024-03-01 to stable/2024-09-01.

* Added tag for 2024-09-01 in readme file

* fix validation error

* fix lint diff error - rleationTypes

* suppress addtional properties in alertRules

* update the common type version to resolve go-sdk error

* fix prettier error

* fix the model validation error

* fix the prettier error

* Watchlists 2024-09-01 (#31040)

* Watchlists 2024-09-01

* Fix errors

* prettier run

* address pr feedback

* address pr feedback

* Add stable version for Connectordefinitions API (#30812)

* add stable version for definition API

* update readme file

* update readme file

* update cSpell

* Create sdk-suppressions.yaml

---------

Co-authored-by: Yuchao Yan <yuchaoyan@microsoft.com>

* Add new kind (RestApiPoller) to DataConnectors STABLE  API (#30813)

* add restApiPoller kind to dataConnectors

* exe prettier

* fix prettier check (#31449)

* Update readme.python.md

* Update sdk-suppressions.yaml

* Fix some typos in DataConnectors and DataConnectorDefinitions APIs (#31581)

* Update sdk-suppressions.yaml

* DataConnectors - MDTI and PMDTI connectors 2024-09-01 (#31491)

* fix prettier check

* mdti stable swagger

* pmdti stable swagger

* fix example name

* fix pmdti swagger

* add pmdti examples

* fix spellcheck

* fix enums

* prettier fix

* update lookback

* lookback period for pmdti

* fix date-time format in examples

* minor format fix

---------

Co-authored-by: xuhumsft <116764429+xuhumsft@users.noreply.github.com>

---------

Co-authored-by: nbatyrbb <115049492+nbatyrbb@users.noreply.github.com>
Co-authored-by: sagamzu <52034287+sagamzu@users.noreply.github.com>
Co-authored-by: Yuchao Yan <yuchaoyan@microsoft.com>
Co-authored-by: Chenjie Shi <tadelesh.shi@live.cn>
Co-authored-by: ni-bhandari <120053375+ni-bhandari@users.noreply.github.com>
pjpatel12 pushed a commit to pjpatel12/azure-rest-api-specs that referenced this pull request Apr 29, 2025
…zure#30800)

* Copy files from stable/2024-03-01

Copied the files in a separate commit.
This allows reviewers to easily diff subsequent changes against the previous spec.

* Update version to stable/2024-09-01

Updated the API version from stable/2024-03-01 to stable/2024-09-01.

* Added tag for 2024-09-01 in readme file

* fix validation error

* fix lint diff error - rleationTypes

* suppress addtional properties in alertRules

* update the common type version to resolve go-sdk error

* fix prettier error

* fix the model validation error

* fix the prettier error

* Watchlists 2024-09-01 (Azure#31040)

* Watchlists 2024-09-01

* Fix errors

* prettier run

* address pr feedback

* address pr feedback

* Add stable version for Connectordefinitions API (Azure#30812)

* add stable version for definition API

* update readme file

* update readme file

* update cSpell

* Create sdk-suppressions.yaml

---------

Co-authored-by: Yuchao Yan <yuchaoyan@microsoft.com>

* Add new kind (RestApiPoller) to DataConnectors STABLE  API (Azure#30813)

* add restApiPoller kind to dataConnectors

* exe prettier

* fix prettier check (Azure#31449)

* Update readme.python.md

* Update sdk-suppressions.yaml

* Fix some typos in DataConnectors and DataConnectorDefinitions APIs (Azure#31581)

* Update sdk-suppressions.yaml

* DataConnectors - MDTI and PMDTI connectors 2024-09-01 (Azure#31491)

* fix prettier check

* mdti stable swagger

* pmdti stable swagger

* fix example name

* fix pmdti swagger

* add pmdti examples

* fix spellcheck

* fix enums

* prettier fix

* update lookback

* lookback period for pmdti

* fix date-time format in examples

* minor format fix

---------

Co-authored-by: xuhumsft <116764429+xuhumsft@users.noreply.github.com>

---------

Co-authored-by: nbatyrbb <115049492+nbatyrbb@users.noreply.github.com>
Co-authored-by: sagamzu <52034287+sagamzu@users.noreply.github.com>
Co-authored-by: Yuchao Yan <yuchaoyan@microsoft.com>
Co-authored-by: Chenjie Shi <tadelesh.shi@live.cn>
Co-authored-by: ni-bhandari <120053375+ni-bhandari@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants