Skip to content

Bump, oxide.json to omicron:1848d183#1085

Merged
ahl merged 20 commits intomainfrom
integration
May 7, 2025
Merged

Bump, oxide.json to omicron:1848d183#1085
ahl merged 20 commits intomainfrom
integration

Conversation

@oxide-reflector-bot
Copy link
Copy Markdown
Contributor

@oxide-reflector-bot oxide-reflector-bot Bot commented Apr 23, 2025

Generated code against nexus.json 1848d183

CLI docs updated against the updated CLI

@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to e4af3302, oxide.json to omicron:4235bfb1 Bump dependencies, progenitor to e4af3302, oxide.json to omicron:26a16838 Apr 24, 2025
@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to e4af3302, oxide.json to omicron:26a16838 Bump dependencies, progenitor to 6f471919, oxide.json to omicron:26a16838 Apr 28, 2025
@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to 6f471919, oxide.json to omicron:26a16838 Bump dependencies, progenitor to c57ee535, oxide.json to omicron:26a16838 Apr 28, 2025
@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to c57ee535, oxide.json to omicron:26a16838 Bump dependencies, progenitor to c57ee535, oxide.json to omicron:21b35cb7 Apr 28, 2025
@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to c57ee535, oxide.json to omicron:21b35cb7 Bump dependencies, progenitor to 77bcb82c, oxide.json to omicron:21b35cb7 Apr 30, 2025
@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to 77bcb82c, oxide.json to omicron:21b35cb7 Bump dependencies, progenitor to 77bcb82c, oxide.json to omicron:f60313ab May 1, 2025
@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to 77bcb82c, oxide.json to omicron:f60313ab Bump dependencies, progenitor to fd3a8944, oxide.json to omicron:f60313ab May 5, 2025
@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to fd3a8944, oxide.json to omicron:f60313ab Bump dependencies, progenitor to fd3a8944, oxide.json to omicron:1848d183 May 6, 2025
oxide vpc firewall-rules update
oxide vpc router route create
oxide vpc router route update
oxide alert receiver webhook create
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is due to secrets: Vec<String> being required in the body. We can/should update progenitor CLI to turn this into an arg that may be multiply specified e.g. --secret x --secret y

Comment thread cli/src/cli_builder.rs
Comment on lines +618 to +634
// Alert subcommands
CliCommand::WebhookEventClassList => Some("alert class list"),
CliCommand::WebhookReceiverList => Some("alert receiver list"),
CliCommand::WebhookDeliveryList => Some("alert receiver log"),
CliCommand::WebhookReceiverProbe => Some("alert receiver probe"),
CliCommand::WebhookDeliveryResend => Some("alert receiver resend"),
CliCommand::WebhookReceiverView => Some("alert receiver view"),
CliCommand::WebhookReceiverDelete => Some("alert receiver delete"),
CliCommand::WebhookReceiverSubscriptionAdd => Some("alert receiver subscribe"),
CliCommand::WebhookReceiverSubscriptionRemove => Some("alert receiver unsubscribe"),

// Webhook specific subcommands (including secret management)
CliCommand::WebhookReceiverCreate => Some("alert receiver webhook create"),
CliCommand::WebhookReceiverUpdate => Some("alert receiver webhook update"),
CliCommand::WebhookSecretsList => Some("alert receiver webhook secret list"),
CliCommand::WebhookSecretsAdd => Some("alert receiver webhook secret add"),
CliCommand::WebhookSecretsDelete => Some("alert receiver webhook secret delete"),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@hawkw and @david-crespo would appreciate your review here

Copy link
Copy Markdown
Contributor

@david-crespo david-crespo May 7, 2025

Choose a reason for hiding this comment

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

I still feel kind of funny about the relationship between receivers and the webhook kind of receiver. I guess I get why the webhook subcommands are separate — webhook receiver create and update are specific to webhooks, while view and delete aren't (at least in their inputs — the output of view would have to be a union of the different types of receiver if there were more). It still doesn't feel quite natural or obvious, but maybe I just need to think about it more.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree that it's a little speculative. My suggestion is that we pick something and hope it works out, but intend to reevaluate once we have a second kind of receiver.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMHO this feels right to me. I would expect the view to be a union of the various receiver types, while the mutation of the receiver is receiver-type-specific, with the exception of subscription management (which should apply to all receivers). Potentially, we might also add API methods to view/list receivers of a specific type only, with the view 404ing if the receiver is another type, but I think that would be more for the benefit of the web UI if it wants to have e.g. webhook-specific pages --- in the CLI, we can just always do the one that returns a union of receiver types and just display whatever comes back...

In the case of secrets specifically, I think it's possible that other receivers may also have a notion of secrets (perhaps you'd want to sign email alerts...) but others may not (a first-party Slack integration would probably have a Slack API key instead of an arbitrary string shared secret) so the semantics might differ substantially.

@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to fd3a8944, oxide.json to omicron:1848d183 Bump dependencies, progenitor to 247e617a, oxide.json to omicron:1848d183 May 7, 2025
@oxide-reflector-bot oxide-reflector-bot Bot changed the title Bump dependencies, progenitor to 247e617a, oxide.json to omicron:1848d183 Bump, oxide.json to omicron:1848d183 May 7, 2025
@ahl ahl merged commit ca173a5 into main May 7, 2025
16 checks passed
@ahl ahl deleted the integration branch May 7, 2025 18:02
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