Azure / azure-openapi-validator

Azure Open API Validator
MIT License
47 stars 46 forks source link

PATCH operation shouldn't be required for proxy resources with no updatable properties #719

Open TimLovellSmith opened 3 months ago

TimLovellSmith commented 3 months ago

Describe the bug

For this typespec

@armResourceOperations(TaskHub)
interface TaskHubs {
  @doc("Get a Task Hub")
  get is ArmResourceRead<TaskHub>;
  @doc("Create or update a Task Hub")
  createOrUpdate is ArmResourceCreateOrReplaceSync<TaskHub>;
  @doc("Delete a Task Hub")
  delete is ArmResourceDeleteSync<TaskHub>;
  @doc("List Task Hubs")
  listByParent is ArmResourceListByParent<TaskHub>;
}

@doc("An Task Hub resource belonging to the namespace.")
@parentResource(Namespace)
model TaskHub is ProxyResource<TaskHubProperties> {
  @doc("Task Hub name")
  @key("taskHubName")
  @pattern("^[a-zA-Z0-9-]{3,64}$")
  @segment("taskHubs")
  @path
  @visibility("read", "create")
  name: string;
}

@doc("The properties of Task Hub")
model TaskHubProperties {
  @visibility("read")
  @doc("The status of the last operation.")
  provisioningState?: ProvisioningState;
}

Apparently, the linter complains that TaskHub needs a PATCH operation, even though it has no writable properties.

"OpenApi specification document is missing 'PATCH' actions in 'namespaces/taskhubs' resource type. Resource type having 'Default' routing type must have GET, PUT, PATCH, DELETE & LIST operations."

To Reproduce Should be possible to have a minimal repro based on the above or private repo file specification/durabletask/DurableTask.Management/main.tsp

Expected behavior In this case there's no good reason to support PATCH, since there are no mutable properties. And in fact, other linter rules agree, because if you try to add the Update/Patch support, you will get errors about using an empty payload (assuming you take the logical approach of having an empty payload)

Screenshots ?

Desktop (please complete the following information): ?

Additional context A productivity pain point for teams onboarding new resource types.

mikeharder commented 3 months ago

@markcowl: What do you think?

rkmanda commented 3 months ago

Agree that this should only be a warning at best for proxy resources