-
Notifications
You must be signed in to change notification settings - Fork 45
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
[Integration][Octopus] Added integration with support for Spaces Project, Release, Deployments and Machines #880
[Integration][Octopus] Added integration with support for Spaces Project, Release, Deployments and Machines #880
Conversation
…nts and Targets (PORT-9398)
…ithub.com:port-labs/ocean into PORT-9398-Add-an-Octopus-Deploy-Ocean-integration Merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some feedback on the mapping and blueprint
[ | ||
{ | ||
"identifier": "octopusDeployProject", | ||
"title": "Octopus Deploy Project", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"title": "Octopus Deploy Project", | |
"title": "Octoput Project", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been updated for all title
"id": { | ||
"type": "string", | ||
"title": "Project ID", | ||
"description": "The unique identifier of the project" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rely on the entity identifier
to identify project so there will be no need for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mapping spaces has helped to solve this. I could now build relationships properly
"lifecycleId": { | ||
"type": "string", | ||
"title": "Lifecycle ID", | ||
"description": "The lifecycle ID associated with the project" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been removed
"spaceId": { | ||
"type": "string", | ||
"title": "Space ID", | ||
"description": "The ID of the space in which the project exists" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be a relation to a space blueprint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been updated
"tenantedDeploymentMode": { | ||
"type": "string", | ||
"title": "Tenanted Deployment Mode", | ||
"description": "The deployment mode regarding tenants" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a defined list for the deployment modes? if so can this be an enum?
created: .Created | ||
deployedBy: .DeployedBy | ||
taskId: .TaskId | ||
projectId: .ProjectId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete this
.SpaceId + '/projects/' + .ProjectId + '/deployments?groupBy=Channel' | ||
relations: | ||
release: .ReleaseId | ||
project: .SpaceId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why spaceId instead of projectId?
hasLatestCalamari: .HasLatestCalamari | ||
communicationStyle: .Endpoint.CommunicationStyle | ||
relations: | ||
project: .SpaceId |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why spaceId?
integrations/octopus/.port/spec.yaml
Outdated
docs: https://docs.getport.io/build-your-software-catalog/sync-data-to-catalog/octopus | ||
features: | ||
- type: exporter | ||
section: DevOps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should go under CICD section
integrations/octopus/.port/spec.yaml
Outdated
- name: octopusUrl | ||
required: true | ||
type: url | ||
description: The base URL of your Octopus Deploy instance. It should include the protocol (e.g., https://). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we have a default url so the user doesn't have to provide if they're using the cloud version?
also can you rename to serverUrl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some design idea on how to restructure the integration and write less code. See the ArgoCD integration code for some context
integrations/octopus/client.py
Outdated
raise | ||
return response.json() | ||
|
||
async def _get_paginated_objects( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to get_paginated_resource()
integrations/octopus/client.py
Outdated
if len(items) < PAGE_SIZE: | ||
break | ||
params["skip"] += PAGE_SIZE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the API provide mechanism to control pagination such as isLastPage or next instead of we handling it manually?
integrations/octopus/client.py
Outdated
async def get_projects(self) -> AsyncGenerator[list[dict[str, Any]], None]: | ||
"""Get all projects.""" | ||
async for projects in self._get_paginated_objects("projects"): | ||
yield projects | ||
|
||
async def get_deployments(self) -> AsyncGenerator[list[dict[str, Any]], None]: | ||
"""Get all deployments.""" | ||
async for deployments in self._get_paginated_objects("deployments"): | ||
yield deployments | ||
|
||
async def get_releases(self) -> AsyncGenerator[list[dict[str, Any]], None]: | ||
"""Get all releases.""" | ||
async for releases in self._get_paginated_objects("releases"): | ||
yield releases | ||
|
||
async def get_targets(self) -> AsyncGenerator[list[dict[str, Any]], None]: | ||
"""Get all targets.""" | ||
async for targets in self._get_paginated_objects("machines"): | ||
yield targets |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the API is organized very well, we wouldn't need to create all these method. we could just reuse the get_paginated_resources directly from the main.py
@ocean.on_resync(ObjectKind.PROJECT)
async def on_resync_projects(kind: str) -> ASYNC_GENERATOR_RESYNC_TYPE:
octopus_client = await init_client()
async for projects in octopus_client.get_paginated_resources(kind):
logger.info(f"Received project batch with {len(projects)} projects")
yield projects
integrations/octopus/main.py
Outdated
@ocean.on_resync(ObjectKind.PROJECT) | ||
async def on_resync_projects(kind: str) -> ASYNC_GENERATOR_RESYNC_TYPE: | ||
octopus_client = await init_client() | ||
async for projects in octopus_client.get_projects(): | ||
logger.info(f"Received project batch with {len(projects)} projects") | ||
yield projects | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you take my comment above in the client.py into consideration, we would have one global resync
@ocean.on_resync()
async def on_global_resync(kind: str) -> ASYNC_GENERATOR_RESYNC_TYPE:
octopus_client = await init_client()
async for resource_batch in octopus_client.get_paginated_resources(kind):
logger.info(f"Received length {len(resource_batch)} of {kind} ")
yield resource_batch
integrations/octopus/main.py
Outdated
@ocean.on_resync(ObjectKind.DEPLOYMENT) | ||
async def on_resync_deployments(kind: str) -> ASYNC_GENERATOR_RESYNC_TYPE: | ||
octopus_client = await init_client() | ||
async for deployments in octopus_client.get_deployments(): | ||
logger.info(f"Received deployment batch with {len(deployments)} deployments") | ||
yield deployments | ||
|
||
|
||
@ocean.on_resync(ObjectKind.RELEASE) | ||
async def on_resync_releases(kind: str) -> ASYNC_GENERATOR_RESYNC_TYPE: | ||
octopus_client = await init_client() | ||
async for releases in octopus_client.get_releases(): | ||
logger.info(f"Received release batch with {len(releases)} releases") | ||
yield releases | ||
|
||
|
||
@ocean.on_resync(ObjectKind.TARGET) | ||
async def on_resync_machines(kind: str) -> ASYNC_GENERATOR_RESYNC_TYPE: | ||
octopus_client = await init_client() | ||
async for machines in octopus_client.get_targets(): | ||
logger.info(f"Received machine batch with {len(machines)} machines") | ||
yield machines | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need all these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some comments, looks great overall
integrations/octopus/main.py
Outdated
ocean.integration_config["octopus_api_key"], | ||
ocean.integration_config["server_url"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reorder the client constructor such that the server_url comes first, followed by the api_key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
[ | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which json formatter are you using? this one seems to have a lot of tab indentation space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The core lint requires four spaces instead of two to be able to pass the lint. I updated to four spaces because of the lint requirement
"type": "string", | ||
"title": "Space URL", | ||
"format": "url", | ||
"description": "The name of the space" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix the description to match the property name. in this case, The Link to the Space in Octopus Deploy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
"deployedBy": { | ||
"type": "string", | ||
"title": "Deployed By", | ||
"description": "The user or system that performed the deployment" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does the deployedBy looks like? a string username or an email?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a string of username
entity: | ||
mappings: | ||
identifier: .Id | ||
title: ".Version + \"(\" + .ProjectId + \")\"" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverse it to {projectId-version}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
integrations/octopus/client.py
Outdated
|
||
class OctopusClient: | ||
def __init__(self, octopus_api_key: str, octopus_url: str) -> None: | ||
self.octopus_url = f"{octopus_url.rstrip('/')}/api/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update this to server url
integrations/octopus/client.py
Outdated
endpoint, json_data=subscription_data, method="POST" | ||
) | ||
|
||
async def create_subscription(self, app_host: str) -> dict[str, Any]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to create_webhook_subscription
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has been renamed
integrations/octopus/client.py
Outdated
await self._create_subscription(space["Id"], app_host) | ||
return {"ok": True} | ||
|
||
async def get_subscriptions(self) -> list[dict[str, Any]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_webhook_subscriptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
|
||
async def setup_application() -> None: | ||
app_host = ocean.integration_config.get("app_host") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're using the app_host variable just once so you can access it directly as
if not ocean.integration_config.get("app_host"):
do something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am using it three times actually. Line 35, 47 and 51
integrations/octopus/main.py
Outdated
payload = data.get("Payload", {}).get("Event", {}) | ||
related_document_ids = payload.get("RelatedDocumentIds", []) | ||
event_category = payload.get("Category", "") | ||
action = "unregister" if "Deleted" in event_category else "register" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you share a sample of the webhook event payload so we understand the logic below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a sample event for the project creation. It is generally same format with other events
{
"Timestamp": "2024-08-06T17:27:55.333+00:00",
"EventType": "SubscriptionPayload",
"Payload": {
"ServerUri": "https://testport.octopus.app",
"ServerAuditUri": "https://testport.octopus.app/app#/Spaces-1/configuration/audit?from=2024-08-06T17%3a27%3a24.%2b00%3a00&to=2024-08-06T17%3a27%3a54.%2b00%3a00",
"BatchProcessingDate": "2024-08-06T17:27:54.670+00:00",
"Subscription": {
"Id": "Subscriptions-4",
"Name": "Port Subscription",
"Type": "Event",
"IsDisabled": false,
"EventNotificationSubscription": {
"Filter": {
"Users": [],
"Projects": [],
"ProjectGroups": [],
"Environments": [],
"EventGroups": [],
"EventCategories": [],
"EventAgents": [],
"Tenants": [],
"Tags": [],
"DocumentTypes": []
},
"EmailTeams": [],
"EmailFrequencyPeriod": "00:00:00",
"EmailPriority": "Normal",
"EmailDigestLastProcessed": null,
"EmailDigestLastProcessedEventAutoId": null,
"EmailShowDatesInTimeZoneId": null,
"WebhookURI": "https://8bc2-102-89-22-116.ngrok-free.app/integration/webhook",
"WebhookTeams": [],
"WebhookTimeout": "00:00:50",
"WebhookHeaderKey": null,
"WebhookHeaderValue": null,
"WebhookLastProcessed": "2024-08-06T17:27:24.568+00:00",
"WebhookLastProcessedEventAutoId": 12157
},
"SpaceId": "Spaces-1",
"Links": {
"Self": "/api/Spaces-1/subscriptions/Subscriptions-4"
}
},
"Event": {
"Id": "Events-12290",
"RelatedDocumentIds": [
"Projects-3"
],
"Category": "Created",
"UserId": "Users-21",
"Username": "explorer3107@gmail.com",
"IsService": false,
"IdentityEstablishedWith": "Session cookie",
"UserAgent": "OctopusClient-js/2024.3.8427",
"Occurred": "2024-08-06T17:27:38.970+00:00",
"Message": "Project Newest Project was created",
"MessageHtml": "Project <a href='#/projects/Projects-3'>Newest Project</a> was created",
"MessageReferences": [
{
"ReferencedDocumentId": "Projects-3",
"StartIndex": 8,
"Length": 14
}
],
"Comments": null,
"Details": null,
"ChangeDetails": {
"DocumentContext": {
"SpaceId": "Spaces-1",
"Id": "Projects-3",
"Name": "Newest Project",
"Slug": "newest-project",
"Description": "Just another test",
"ExecuteDeploymentsOnResilientPipeline": null,
"IsDisabled": false,
"VariableSetId": "variableset-Projects-3",
"DeploymentProcessId": "deploymentprocess-Projects-3",
"DeploymentSettingsId": "deploymentsettings-Projects-3",
"ClonedFromProjectId": null,
"ProjectGroupId": "ProjectGroups-3",
"LifecycleId": "Lifecycles-3",
"AutoCreateRelease": false,
"PersistenceSettings": {
"Type": "Database"
},
"DynamicEnvironmentSettings": {
"ProvisioningRunbook": null,
"DeprovisioningRunbook": null
},
"DiscreteChannelRelease": false,
"IncludedLibraryVariableSetIds": [],
"UsedPackages": [],
"TenantedDeploymentMode": "Untenanted",
"Templates": [],
"ReleaseCreationStrategy": {
"ReleaseCreationPackage": null,
"ChannelId": null
},
"AutoDeployReleaseOverrides": [],
"ExtensionSettings": [],
"AllowIgnoreChannelRules": true,
"DataVersion": null
},
"Differences": []
},
"IpAddress": "102.89.34.130",
"SpaceId": "Spaces-1",
"Links": {
"Self": "/api/events/Events-12290"
}
},
"BatchId": "3531f2c3-102a-4e18-9d6f-5dacf4d7ae58",
"TotalEventsInBatch": 6,
"EventNumberInBatch": 1
}
}
"""Create a new subscription.""" | ||
for space in await self.get_all_spaces(): | ||
await self._create_subscription(space["Id"], app_host) | ||
return {"ok": True} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of manually setting ok to True, can we rely on the response from the _create_subscription
. And maybe keep track of which spaces we were able to create subscription for and which one we didn't?
…ithub.com:port-labs/ocean into PORT-9398-Add-an-Octopus-Deploy-Ocean-integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left comments, will re-review after you change these ones
Co-authored-by: Matan <51418643+matan84@users.noreply.github.com>
Co-authored-by: Matan <51418643+matan84@users.noreply.github.com>
Co-authored-by: Matan <51418643+matan84@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This follows the present achievable in other integrations
Just the latest 100 release will be ingested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one comment
…ttps://github.com/port-labs/ocean into PORT-9398-Add-an-Octopus-Deploy-Ocean-integration
…ttps://github.com/port-labs/ocean into PORT-9398-Add-an-Octopus-Deploy-Ocean-integration
Updated the limitation implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left 2 comments
Co-authored-by: Matan <51418643+matan84@users.noreply.github.com>
Co-authored-by: Matan <51418643+matan84@users.noreply.github.com>
Updated Naming
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
What -
Introducing a new integration with Octopus Deploy, which allows for synchronization of space, project, release, deployment, and machine data into our platform.
Why -
To provide users with the ability to visualize and manage their deployment processes within our platform, enhancing project tracking and decision-making.
How -
Type of change
Please leave one option from the following and delete the rest:
Screenshots
Include screenshots from your environment showing how the resources of the integration will look.
API Documentation