Skip to content
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

Description for icmpCode is inaccurate for protocol value relation #5479

Open
ps-mattstuart opened this issue Sep 22, 2024 · 1 comment
Open
Labels
docs/providers kind/bug Some behavior is incorrect or out of spec kind/codegen Issue relates to upstream auto codgens -- Python SDK, Bridge, Provider(s)

Comments

@ps-mattstuart
Copy link

File: themes/default/content/registry/packages/aws/api-docs/ec2/networkaclrule/_index.md

Existing:

[icmpCode](https://www.pulumi.com/registry/packages/aws/api-docs/ec2/networkaclrule/#state_icmpcode_nodejs) Changes to this property will trigger replacement.
number
ICMP protocol: The ICMP code. Required if specifying ICMP for the protocolE.g., -1

NOTE: If the value of protocol is -1 or all, the from_port and to_port values will be ignored and the rule will apply to all ports.

NOTE: If the value of icmp_type is -1 (which results in a wildcard ICMP type), the icmp_code must also be set to -1 (wildcard ICMP code).

Note: For more information on ICMP types and codes, see here: https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml

Issue Line:
ICMP protocol: The ICMP code. Required if specifying ICMP for the protocolE.g., -1

The note about "specifying ICMP for the protocolE.G., -1" is inaccurate in this context. It should instead reach "specifying ICMP for the protocol (icmp)" or similar to indicate that the necessary protocol for ICMP only traffic is supposed to be icmp and not -1 which forces all traffic. The list of available protocols isn't defined and this text makes it potentially confusing.

@pulumi-bot pulumi-bot added the needs-triage Needs attention from the triage team label Sep 22, 2024
@github-project-automation github-project-automation bot moved this to 🤔 Triage in Docs 📚 Sep 22, 2024
@thoward thoward added area/docs Hand-written documentation. For api docs, see area/api-docs. docs/providers kind/documentation Improvements or additions to documentation labels Sep 25, 2024
@thoward thoward moved this from 🤔 Triage to 🧳 Backlog in Docs 📚 Sep 25, 2024
@thoward thoward removed the needs-triage Needs attention from the triage team label Sep 25, 2024
@thoward
Copy link
Contributor

thoward commented Sep 25, 2024

Thanks for reporting this! I looked over the documentation and it seems that we have a Markdown rendering bug here that is making this page accidentally include a note about the protocol value inside of the description for the icmp_code value. Unfortunately, since both have special handling of the value -1, that makes it extra confusing as a reader.

We'll file a ticket with the appropriate team/codebase to address this rendering problem.

Regarding the accuracy of the information, it is accurate, but the rendering is putting it in the wrong order, which makes it very confusing as to which option is being talked about. What it's intending to describe is:

The value protocol refers to the name of the protocol, e.g. icmp and setting a value of -1 does indicate "all protocols".

However, icmp_type and icmp_code are referring to options that are only used when protocol is set to icmp. They map to these ICMP parameters: https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml

In this case, a value of -1 for icmp_type means "all ICMP types" and the value of - for icmp_code means "all ICMP codes for the specified ICMP type" respectively (here's the AWS API documentation for these values: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_IcmpTypeCode.html ).

We'll make sure to get this sorted out and sorry for the confusion!

@thoward thoward added kind/bug Some behavior is incorrect or out of spec area/registry kind/codegen Issue relates to upstream auto codgens -- Python SDK, Bridge, Provider(s) and removed area/docs Hand-written documentation. For api docs, see area/api-docs. kind/documentation Improvements or additions to documentation labels Sep 25, 2024
@thoward thoward removed this from Docs 📚 Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs/providers kind/bug Some behavior is incorrect or out of spec kind/codegen Issue relates to upstream auto codgens -- Python SDK, Bridge, Provider(s)
Projects
None yet
Development

No branches or pull requests

4 participants