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.

Sign up for GitHub

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

Jump to bottom

azurerm_express_route_port: Added macsec_sci_state and additional ciphers #13006

Closed
wants to merge 1 commit into from
Closed

azurerm_express_route_port: Added macsec_sci_state and additional ciphers #13006

wants to merge 1 commit into from

Conversation

tiwood
Copy link
Contributor

@tiwood tiwood commented Aug 16, 2021

New attribute: macsec_sci_state, which sets https://docs.microsoft.com/en-us/rest/api/expressroute/express-route-ports/create-or-update#expressroutelinkmacsecscistate

Added missing XPN ciphers.

This has to be merged first: #13002

@tiwood tiwood force-pushed the r/azurerm_express_route_port/CiphersAndSCI branch from 09461b2 to 69a4bf1 Compare August 17, 2021 10:40
katbyte
katbyte requested changes Aug 18, 2021
View reviewed changes
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Thanks @tiwood - looks good aside from one comment about the schema and missing docs

internal/services/network/express_route_port_resource.go
@@ -64,6 +57,11 @@ var expressRoutePortSchema = &pluginsdk.Schema{
Optional: true,
ValidateFunc: validation.StringIsNotEmpty,
},
"macsec_sci_state": {
Copy link
Collaborator

@katbyte katbyte Aug 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

state doesn't make much sense here? should this be:

Suggested change
"macsec_sci_state": {
"macsec_sci_enabled": {

@katbyte katbyte added this to the v2.73.0 milestone Aug 18, 2021
@katbyte katbyte modified the milestones: v2.73.0, v2.74.0, v2.75.0 Aug 20, 2021
@catriona-m catriona-m self-assigned this Sep 1, 2021
@tiwood
Copy link
Contributor Author

tiwood commented Sep 1, 2021

Just to let you know, there currently is an issue with the Azure API - it seems the SCI State is not reported back, which kind of blocks us here.

@katbyte katbyte modified the milestones: v2.75.0, v2.76.0 Sep 2, 2021
@katbyte
Copy link
Collaborator

katbyte commented Sep 9, 2021

@tiwood - should we close this until the API has been fixed? it can be reopened once thats happened

@katbyte katbyte modified the milestones: v2.76.0, v2.77.0 Sep 10, 2021
@katbyte
Copy link
Collaborator

katbyte commented Sep 16, 2021

Going to close this for now @tiwood - we can reopen it once the bug is fixed. thanks!

@katbyte katbyte closed this Sep 16, 2021
@github-actions
Copy link

This functionality has been released in v2.77.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@katbyte katbyte katbyte requested changes

Assignees

@catriona-m catriona-m

Labels
enhancement service/express-route size/S
Projects
None yet
Milestone
v2.77.0
Development

Successfully merging this pull request may close these issues.
3 participants
@tiwood @katbyte @catriona-m