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

VPC endpoint state value parsing is incorrect #619

Open
imp opened this issue Sep 19, 2022 · 7 comments
Open

VPC endpoint state value parsing is incorrect #619

imp opened this issue Sep 19, 2022 · 7 comments
Labels
bug This issue is a bug. ec2 p3 This is a minor priority issue service-api This issue is due to a problem in a service API, not the SDK implementation.

Comments

@imp
Copy link
Contributor

imp commented Sep 19, 2022

Describe the bug

VPC endpoint state values when parsed from their text representation are incorrect. The reason is that code expect them to be in PascalCase, while in fact they are camelCase. As a result all the values are getting classified as State::Unknown("xxx").

For example

pendingAcceptance is expected to be PendingAcceptance
available is expected to be Available
etc

Expected Behavior

When running reproduction code below I expect the next output

Some(
    PendingAcceptance,
),
Some(
    Available
)

Current Behavior

Instead every possible VPC endpoint state in describe_vpc_endpoints() is showed as State::Unknown("text")

Some(
    Unknown(
        "pendingAcceptance",
    ),
)
Some(
    Unknown(
        "available",
    ),
)

Reproduction Steps

If you have at least one VPC endpoint

use aws_sdk_ec2 as ec2;

#[tokio::main]
async fn main() -> anyhow::Result<()> {
    let shared_config = aws_config::load_from_env().await;
    ec2::Client::new(&shared_config)
        .describe_vpc_endpoints()
        .send()
        .await?
        .vpc_endpoints
        .unwrap_or_default()
        .into_iter()
        .for_each(|ep| println!("{:#?}", ep.state()));
    Ok(())
}

Possible Solution

diff --git a/sdk/ec2/src/model.rs b/sdk/ec2/src/model.rs
index 916c9ee62..30ba20e6a 100644
--- a/sdk/ec2/src/model.rs
+++ b/sdk/ec2/src/model.rs
@@ -68004,14 +68004,14 @@ pub enum State {
 impl std::convert::From<&str> for State {
     fn from(s: &str) -> Self {
         match s {
-            "Available" => State::Available,
-            "Deleted" => State::Deleted,
-            "Deleting" => State::Deleting,
-            "Expired" => State::Expired,
-            "Failed" => State::Failed,
-            "Pending" => State::Pending,
-            "PendingAcceptance" => State::PendingAcceptance,
-            "Rejected" => State::Rejected,
+            "available" => State::Available,
+            "deleted" => State::Deleted,
+            "deleting" => State::Deleting,
+            "expired" => State::Expired,
+            "failed" => State::Failed,
+            "pending" => State::Pending,
+            "pendingAcceptance" => State::PendingAcceptance,
+            "rejected" => State::Rejected,
             other => State::Unknown(other.to_owned()),
         }
     }
@@ -68027,28 +68027,28 @@ impl State {
     /// Returns the `&str` value of the enum member.
     pub fn as_str(&self) -> &str {
         match self {
-            State::Available => "Available",
-            State::Deleted => "Deleted",
-            State::Deleting => "Deleting",
-            State::Expired => "Expired",
-            State::Failed => "Failed",
-            State::Pending => "Pending",
-            State::PendingAcceptance => "PendingAcceptance",
-            State::Rejected => "Rejected",
+            State::Available => "available",
+            State::Deleted => "deleted",
+            State::Deleting => "deleting",
+            State::Expired => "expired",
+            State::Failed => "failed",
+            State::Pending => "pending",
+            State::PendingAcceptance => "pendingAcceptance",
+            State::Rejected => "rejected",
             State::Unknown(s) => s.as_ref(),
         }
     }
     /// Returns all the `&str` values of the enum members.
     pub fn values() -> &'static [&'static str] {
         &[
-            "Available",
-            "Deleted",
-            "Deleting",
-            "Expired",
-            "Failed",
-            "Pending",
-            "PendingAcceptance",
-            "Rejected",
+            "available",
+            "deleted",
+            "deleting",
+            "expired",
+            "failed",
+            "pending",
+            "pendingAcceptance",
+            "rejected",
         ]
     }
 }

Additional Information/Context

No response

Version

├── aws-config v0.48.0
│   ├── aws-http v0.48.0
│   │   ├── aws-smithy-http v0.48.0
│   │   │   ├── aws-smithy-types v0.48.0
│   │   ├── aws-smithy-types v0.48.0 (*)
│   │   ├── aws-types v0.48.0
│   │   │   ├── aws-smithy-async v0.48.0
│   │   │   ├── aws-smithy-client v0.48.0
│   │   │   │   ├── aws-smithy-async v0.48.0 (*)
│   │   │   │   ├── aws-smithy-http v0.48.0 (*)
│   │   │   │   ├── aws-smithy-http-tower v0.48.0
│   │   │   │   │   ├── aws-smithy-http v0.48.0 (*)
│   │   │   │   ├── aws-smithy-types v0.48.0 (*)
│   │   │   ├── aws-smithy-http v0.48.0 (*)
│   │   │   ├── aws-smithy-types v0.48.0 (*)
│   ├── aws-sdk-sso v0.18.0
│   │   ├── aws-endpoint v0.48.0
│   │   │   ├── aws-smithy-http v0.48.0 (*)
│   │   │   ├── aws-smithy-types v0.48.0 (*)
│   │   │   ├── aws-types v0.48.0 (*)
│   │   ├── aws-http v0.48.0 (*)
│   │   ├── aws-sig-auth v0.48.0
│   │   │   ├── aws-sigv4 v0.48.0
│   │   │   │   ├── aws-smithy-http v0.48.0 (*)
│   │   │   ├── aws-smithy-http v0.48.0 (*)
│   │   │   ├── aws-types v0.48.0 (*)
│   │   ├── aws-smithy-async v0.48.0 (*)
│   │   ├── aws-smithy-client v0.48.0 (*)
│   │   ├── aws-smithy-http v0.48.0 (*)
│   │   ├── aws-smithy-http-tower v0.48.0 (*)
│   │   ├── aws-smithy-json v0.48.0
│   │   │   └── aws-smithy-types v0.48.0 (*)
│   │   ├── aws-smithy-types v0.48.0 (*)
│   │   ├── aws-types v0.48.0 (*)
│   ├── aws-sdk-sts v0.18.0
│   │   ├── aws-endpoint v0.48.0 (*)
│   │   ├── aws-http v0.48.0 (*)
│   │   ├── aws-sig-auth v0.48.0 (*)
│   │   ├── aws-smithy-async v0.48.0 (*)
│   │   ├── aws-smithy-client v0.48.0 (*)
│   │   ├── aws-smithy-http v0.48.0 (*)
│   │   ├── aws-smithy-http-tower v0.48.0 (*)
│   │   ├── aws-smithy-query v0.48.0
│   │   │   ├── aws-smithy-types v0.48.0 (*)
│   │   ├── aws-smithy-types v0.48.0 (*)
│   │   ├── aws-smithy-xml v0.48.0
│   │   ├── aws-types v0.48.0 (*)
│   ├── aws-smithy-async v0.48.0 (*)
│   ├── aws-smithy-client v0.48.0 (*)
│   ├── aws-smithy-http v0.48.0 (*)
│   ├── aws-smithy-http-tower v0.48.0 (*)
│   ├── aws-smithy-json v0.48.0 (*)
│   ├── aws-smithy-types v0.48.0 (*)
│   ├── aws-types v0.48.0 (*)
├── aws-sdk-ec2 v0.18.0
│   ├── aws-endpoint v0.48.0 (*)
│   ├── aws-http v0.48.0 (*)
│   ├── aws-sig-auth v0.48.0 (*)
│   ├── aws-smithy-async v0.48.0 (*)
│   ├── aws-smithy-client v0.48.0 (*)
│   ├── aws-smithy-http v0.48.0 (*)
│   ├── aws-smithy-http-tower v0.48.0 (*)
│   ├── aws-smithy-query v0.48.0 (*)
│   ├── aws-smithy-types v0.48.0 (*)
│   ├── aws-smithy-xml v0.48.0 (*)
│   ├── aws-types v0.48.0 (*)
├── aws-types v0.48.0 (*)


### Environment details (OS name and version, etc.)

macOS Monterey

### Logs

_No response_
@imp imp added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 19, 2022
@jdisanti
Copy link
Contributor

It looks like this same issue is happening for the Go V2 SDK, and a similar problem is happening to SSM in the Rust SDK. There seems to be a pattern among these enums.

Internal tracking ID: P66381300

@jdisanti jdisanti removed the needs-triage This issue or PR still needs to be triaged. label Sep 19, 2022
@jdisanti jdisanti added the service-api This issue is due to a problem in a service API, not the SDK implementation. label Sep 30, 2022
@jmklix
Copy link
Member

jmklix commented Oct 3, 2022

Opened an issue in the aws/aws-sdk which is used to track issues across multiple sdks. Closing this issue

@jmklix jmklix closed this as completed Oct 3, 2022
@github-actions
Copy link

github-actions bot commented Oct 3, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@jubrad
Copy link

jubrad commented May 22, 2023

This is pretty annoying issue, largely due to the fact that UnknownVariantValue is closed off and only has private fields. Imagine wanting to match on vpce.state. This is how I'm currently doing this...

match vpce.state.unwrap() {
  // match expected behavior
  Available => { do_foo() },
  Pending => { do_bar() },
  ...
  // work around this issue
  Unknown(variant) => {
    let variant = format!("{:?}", variant)};
    match variant.as_str() => {
        "UnknownVariantValue(\"available\")" => { do_foo() },
        "UnknownVariantValue(\"pending\")" => { do_bar() },
        _ => { do_baz() }
    } 
  }
  _ => { do_baz() }
}

Making the value field publicly accessible leads to a much better solution, at least until this is actually fixed. The following is an example. This is no longer tied to a potential change in display functionality for UnkownVariantValue, or the introduction of a new field.

match vpce.state.unwrap() {
  // match expected behavior
  Available => { do_foo() },
  Pending => { do_bar() },
  ...
  // work around this issue
  Unknown(variant) => {
    match variant.value() => {
        "available" => { do_foo() },
        "pending" => { do_bar() },
        _ => { do_baz() }
    } 
  }
  _ => { do_baz() }
}

jubrad added a commit to jubrad/aws-sdk-rust that referenced this issue May 22, 2023
pub value retrieval for UnkownVariantValue

    adds preferred workaround for awslabs#619
@jdisanti
Copy link
Contributor

You shouldn't need to format! the variant to match on it:

match vpce.state.unwrap() {
  // match expected behavior
  Available => { do_foo() },
  Pending => { do_bar() },
  ...
  // work around this issue
  other @ _ => match other.as_str() => {
    "available" => { do_foo() },
    "pending" => { do_bar() },
    _ => { do_baz() }
  }
}

The unknown variant value is intended to not be used directly, as matching on it could lead to your code breaking in the future. For example, if the SDK didn't yet have a new enum value and you matched on the string value directly, that code would break in the future when an enum variant was added to the SDK for that value. By matching on the enum's string value instead of on the unknown variant value, your code is safe from that.

@jubrad
Copy link

jubrad commented May 22, 2023

Yeah, I think I was stuck trying to check against the value in the unknown enum, I hadn't considered just turning unknown enum into a string. Thanks!

@jmklix
Copy link
Member

jmklix commented Oct 30, 2024

Re-opening this issue and closing the tracking issue

P66381300

@jmklix jmklix reopened this Oct 30, 2024
@jmklix jmklix added p3 This is a minor priority issue ec2 labels Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. ec2 p3 This is a minor priority issue service-api This issue is due to a problem in a service API, not the SDK implementation.
Projects
None yet
Development

No branches or pull requests

4 participants