-
Notifications
You must be signed in to change notification settings - Fork 6
feat: Add Service resource support #549
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
base: main
Are you sure you want to change the base?
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. New resources:
Maintainer note: consult the runbook for dealing with any breaking changes. |
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.
Pull Request Overview
This PR adds support for managing Pulumi Cloud Service resources through the provider. Services in Pulumi Cloud allow users to group and organize related stacks and environments.
- Added Service resource with full CRUD operations and service item management
- Implemented comprehensive API client for service operations
- Added complete SDK support across all languages
Reviewed Changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/python/pulumi_pulumiservice/service.py | Python SDK for Service resource with complete property mappings |
| sdk/python/pulumi_pulumiservice/outputs.py | Added ServiceItem output type for Python SDK |
| sdk/python/pulumi_pulumiservice/_inputs.py | Added ServiceItem input type for Python SDK |
| sdk/nodejs/service.ts | Node.js SDK for Service resource with TypeScript definitions |
| sdk/nodejs/types/output.ts | Added ServiceItem interface for Node.js output types |
| sdk/nodejs/types/input.ts | Added ServiceItem interface for Node.js input types |
| sdk/go/pulumiservice/service.go | Go SDK for Service resource with input/output wrappers |
| sdk/go/pulumiservice/pulumiTypes.go | Added ServiceItem type definitions for Go SDK |
| sdk/dotnet/Service.cs | .NET SDK for Service resource implementation |
| sdk/dotnet/Outputs/ServiceItem.cs | .NET output type for ServiceItem |
| sdk/dotnet/Inputs/ServiceItemArgs.cs | .NET input type for ServiceItem |
| provider/pkg/resources/service.go | Core Service resource implementation with CRUD operations |
| provider/pkg/pulumiapi/services.go | HTTP client for Service API operations |
| provider/cmd/pulumi-resource-pulumiservice/schema.json | Schema definitions for Service and ServiceItem types |
| examples/yaml-service/ | YAML example demonstrating Service resource usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
fnune
left a comment
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 only feel entitled to leaving a superficial review, as I haven't conrtibuted to this codebase yet. Leaving it up to others to approve/request changes. (@flostadler?)
examples/main_test.go
Outdated
| // Only set PULUMI_TEST_OWNER if not already set by environment | ||
| if os.Getenv("PULUMI_TEST_OWNER") == "" { | ||
| _ = os.Setenv("PULUMI_TEST_OWNER", "service-provider-test-org") | ||
| } | ||
| _ = os.Setenv("PULUMI_TEST_USE_SERVICE", "true") | ||
| m.Run() |
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.
Nit: assign the result of os.Getenv("...") to variables and pass them down to m.Run()? The comment would then not be necessary.
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.
Nit: seems like an accidental change here.
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.
A Service's main purpose is to group entities within it. Should the example include at least one or two entities?
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 going to add parts that would be part of the service then come back to this PR so we can tie it all together. This makes a lot of sense to show a full working example.
| "willReplaceOnChanges": true | ||
| }, | ||
| "ownerName": { | ||
| "description": "The name of the owner (username or team name).", |
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.
Isn't this only relevant for the team case? For the user case we'd use the current user's username
| "properties": { | ||
| "itemType": { | ||
| "type": "string", | ||
| "description": "The type of item (stack or environment)." |
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 add an enum for this
| "type": "string", | ||
| "description": "The type of item (stack or environment)." | ||
| }, | ||
| "name": { |
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 users have to provide the organization here? A service is already org specific.
What about splitting the name up into separate project and name properties. I find this implicit naming hard to discover 🤔
| "willReplaceOnChanges": true | ||
| }, | ||
| "ownerType": { | ||
| "description": "The type of owner for this service (user or team).", |
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 add an enum 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.
We should probably make it an enum in the swagger.json if that is the case, as neither the docs, or swagger have it that way:
"CreateServiceRequest": {
"type": "object",
"required": [
"description",
"items",
"name",
"ownerName",
"ownerType",
"properties"
],
"properties": {
"ownerType": {
"type": "string",
"description": "the service owner type",
"x-order": 1
},
"ownerName": {
"type": "string",
"description": "the service owner name",
"x-order": 2
},
"name": {
"type": "string",
"description": "the name of the service",
"x-order": 3
},
"description": {
"type": "string",
"description": "an optional description of the service",
"x-order": 4
},
"items": {
"type": "array",
"description": "an optional list of items to add during service creation",
"items": {
"$ref": "#/definitions/AddServiceItem",
"x-pulumi-model-property": {
"usePointerForGo": true
}
},
"x-order": 5
},
"properties": {
"type": "array",
"description": "an optional list of properties to set on the service",
"items": {
"$ref": "#/definitions/ServiceProperty",
"x-pulumi-model-property": {
"usePointerForGo": true
}
},
"x-order": 6
}
},
"description": "Request for creating a new service with an optional list\nof items to populate the service with after creation."
},```
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.
That's a good point, we should add it to the spec as well! Taking a not to make that change
| }, | ||
| "requiredInputs": [ | ||
| "organizationName", | ||
| "ownerType", |
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 about omitting it and defaulting it to the current user?
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.
How do we plan on modeling this against the swagger.json eventually? It seems like we will have to add a bunch of rules for each type do determine what gets left out from the swagger into the main schema.
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 could in theory model this as a one of and using the ownerType as a discriminator
This PR adds support for managing Pulumi Cloud Service resources through the provider. Services in Pulumi Cloud allow users to group and organize related stacks and environments.
- Added `ServiceItem` type definition to support service item management
- Added `Service` resource with full CRUD operations including:
- `organizationName` (required): Organization containing the service
- `ownerType` (required): Type of owner ("user" or "team")
- `ownerName` (required): Name of the owner
- `name` (required): Service name
- `description` (optional): Service description
- `properties` (optional): Key-value properties for the service
- `items` (optional): Array of service items (stacks/environments)
- Created `services.go` with full CRUD operations:
- `CreateService`: POST /api/orgs/{org}/services
- `GetService`: GET /api/orgs/{org}/services/{ownerType}/{ownerName}/{serviceName}
- `UpdateService`: PATCH /api/orgs/{org}/services/{ownerType}/{ownerName}/{serviceName}
- `DeleteService`: DELETE /api/orgs/{org}/services/{ownerType}/{ownerName}/{serviceName}
- `AddServiceItem`: Add items to service
- `RemoveServiceItem`: Remove items from service
- Implemented `PulumiServiceServiceResource` with all required interface methods
- Full support for property mapping and diff calculation
- Reconciliation of service items during updates (add/remove as needed)
- Proper ID format: `{org}/{ownerType}/{ownerName}/{serviceName}`
- Unit tests for CRUD operations
- ID parsing and generation tests
- All tests passing with proper mocking
- YAML example demonstrating service creation with items
- TypeScript example showing programmatic service management
- Updated CHANGELOG.md with new feature entry
- ✅ Provider builds successfully
- ✅ All linting checks pass
- ✅ Unit tests pass (5/5 tests)
- ✅ SDKs generated successfully for all languages (Go, Node.js, Python, .NET, Java)
Fixes #522
41bedfe to
4a5513f
Compare
Co-authored-by: Fausto Núñez Alberro <[email protected]> Co-authored-by: Florian Stadler <[email protected]>
4a5513f to
14e61ac
Compare
Summary
This PR adds support for managing Pulumi Cloud Service resources through the provider. Services in Pulumi Cloud allow users to group and organize related stacks and environments.
Implementation Details
Schema Changes
ServiceItemtype definition to support service item managementServiceresource with full CRUD operations including:organizationName(required): Organization containing the serviceownerType(required): Type of owner ("user" or "team")ownerName(required): Name of the ownername(required): Service namedescription(optional): Service descriptionproperties(optional): Key-value properties for the serviceitems(optional): Array of service items (stacks/environments)API Client
services.gowith full CRUD operations:CreateService: POST /api/orgs/{org}/servicesGetService: GET /api/orgs/{org}/services/{ownerType}/{ownerName}/{serviceName}UpdateService: PATCH /api/orgs/{org}/services/{ownerType}/{ownerName}/{serviceName}DeleteService: DELETE /api/orgs/{org}/services/{ownerType}/{ownerName}/{serviceName}AddServiceItem: Add items to serviceRemoveServiceItem: Remove items from serviceResource Implementation
PulumiServiceServiceResourcewith all required interface methods{org}/{ownerType}/{ownerName}/{serviceName}Testing
Examples
Documentation
Testing Done
Fixes #522