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

Skip tag operations on cloudwatch logs in govcloud partition. #12414

Merged
merged 1 commit into from
May 9, 2017

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Mar 3, 2017

AWS GovCloud doesn't support tag operations on cloudwatch log groups:

± aws logs list-tags-log-group --log-group-name some-log-group
An error occurred (UnknownOperationException) when calling the ListTagsLogGroup operation:

± aws logs tag-log-group --log-group-name some-log-group --tags Name=test
An error occurred (UnknownOperationException) when calling the TagLogGroup operation:

This patch skips adding, removing, and listing tags on cloudwatch log groups when the partition is aws-us-gov.

cc @LinuxBozo

@apparentlymart
Copy link
Contributor

Thanks for this, @jmcarp! I'm going to leave it to another Terraform maintainer to review/test since I don't have a govcloud account to try it with, but I do have a question:

Do you know that this limitation is intentional on the part of AWS? I wonder if this is a bug on their end that could be fixed, rather than adding this special case to Terraform. It seems like a rather arbitrary restriction, but perhaps this sort of additional constraint is normal for GovCloud?

@jmcarp
Copy link
Contributor Author

jmcarp commented Mar 3, 2017

I couldn't find any mention of this limitation in the GovCloud docs, but that's not unusual for GovCloud limitations. I completely agree that this should be fixed on AWS and not terraform, but in my experience, it can take a while for GovCloud to catch up with the main partition. I'll check with AWS support and report back here if I learn anything.

@apparentlymart
Copy link
Contributor

Thanks @jmcarp! If this is indeed something that isn't going to get fixed for a while then working around it in Terraform would make sense, though perhaps we should make Terraform fail with an error message if any tags are provided in config in this case so that Terraform isn't just silently ignoring them, which would be particularly confusing if GovCloud later got support and it looked like a Terraform bug that the tags would not apply.

@jmcarp
Copy link
Contributor Author

jmcarp commented Mar 10, 2017

@apparentlymart @paddyforan: here's what I heard from AWS support:

The tagging issue is a known limitation that the service team is actively working. They expect this feature to be available early Q2.

My team doesn't have an urgent need to update to 0.8.x, so I don't have a strong preference for (a) fixing this patch up and reverting in a few months or (b) doing nothing and waiting for AWS to fix. WDYT?

@realflash
Copy link
Contributor

I would dearly love access to the local data provider so I vote for patch it now, please. Who knows when Amazon will actually deliver this functionality, if they ever do? The number of services in govcloud is about 20% of those in other partitions; being crippled is practically part of its raison d'etre.

@radeksimko
Copy link
Member

Hi 👋
I think it's ok to put workaround in place to make things work for "special" regions.

That said given that there are no guarantees/plans about what is/isn't support in any given region I think it would be better if we just silently ignored UnknownOperationException error code for ListTagsLogGroup + perhaps logged a DEBUG message mentioning that we ignored it, just for potential future debugging purposes.

We can keep (Un)TagLogGroup as these should error out when the user actually does try to define tags in region which doesn't support it.

This is to avoid the need to fix this again and cut release when Amazon starts supporting it in any specific region.

@jmcarp What do you think?

@radeksimko radeksimko added the waiting-response An issue/pull request is waiting for a response from the community label Apr 23, 2017
@paddycarver
Copy link
Contributor

Hi! This issue has been open for over two weeks and waiting response from the community without any activity, so I'm going to close it out. We do this to try to keep our attention on issues that are actively impacting the community, it's not a comment on the issue itself. If there is still interest in this issue, please feel free to reply, and we'll re-open the issue. Thanks!

@paddycarver paddycarver closed this May 8, 2017
@realflash
Copy link
Contributor

What sort of response do you want? I said "I would dearly love access to the local data provider so I vote for patch it now, please. Who knows when Amazon will actually deliver this functionality, if they ever do? The number of services in govcloud is about 20% of those in other partitions; being crippled is practically part of its raison d'etre."

@realflash
Copy link
Contributor

I'm just waiting for it to be fixed - we're stuck on 0.7.2 and not able to progress our usage until it is. If there's something we can contribute to the decision please say so.

@KevinBrooke

@LinuxBozo
Copy link

I think the solution proposed by @radeksimko works all around.

@grubernaut
Copy link
Contributor

Hey all, I'm going to re-open this and we're going to begin working on a fix for this.

I agree with @radeksimko's solution, and think it's the best path forward in the long-run. However, this patch will also solve the issue in the immediate term. Either way, we should fix this issue. I apologize for the confusion here, it should not have been closed.

@grubernaut grubernaut reopened this May 9, 2017
@grubernaut grubernaut removed the waiting-response An issue/pull request is waiting for a response from the community label May 9, 2017
Copy link
Contributor

@grubernaut grubernaut left a comment

Choose a reason for hiding this comment

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

LGTM
This fixes the immediate issue, for now.
We now have access to a govcloud testing account and should be able to start fully enhancing our AWS resources with non-default partition support, and better handle these cases.

@ghost
Copy link

ghost commented Apr 12, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants