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

Add support for more fields to Edge gateway configuration, add multiple subnet support #267

Merged
merged 15 commits into from
Nov 29, 2019

Conversation

Didainius
Copy link
Collaborator

@Didainius Didainius commented Nov 18, 2019

This PR:

Note. Test suite passed on 9.5 and 10.

@Didainius Didainius marked this pull request as ready for review November 19, 2019 07:47
@Didainius Didainius requested review from dataclouder, vbauzys and lvirbalas and removed request for dataclouder and vbauzys November 19, 2019 07:47
@Didainius Didainius self-assigned this Nov 19, 2019
CHANGELOG.md Outdated
* Change to pointers `DistributedRoutingEnabled` in `GatewayConfiguration` and `DistributedInterface` in `NetworkConfiguration`
* Change to pointers `DistributedRoutingEnabled` in `GatewayConfiguration` and
`DistributedInterface` in `NetworkConfiguration`
* Add new fields to type `GatewayConfiguration`: `FipsModeEnabled` -
Copy link
Collaborator

Choose a reason for hiding this comment

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

Says fields in plural, but only a single field is referenced.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed. Fixed.

// rules in the old 1.5 format. The new format does not require to use direction in firewall
// rules. Also, for firewall rules to allow NAT traffic the filter is applied on the original IP
// addresses. Once set to true cannot be reverted back to false.
BackwardCompatibilityMode bool `xml:"BackwardCompatibilityMode,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a note, +1 for this way of adding full docs to fields in types.

Copy link
Contributor

@dataclouder dataclouder left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@vbauzys vbauzys left a comment

Choose a reason for hiding this comment

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

LGTM

// GatewayBackingConfig defines configuration of the vShield edge VM for this gateway. One of:
// compact, full.
GatewayBackingConfig string `xml:"GatewayBackingConfig"`
// GatewayInterfaces holds configuration for
Copy link
Contributor

Choose a reason for hiding this comment

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

unfinished sentece

Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

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

Looks great now!

@Didainius Didainius merged commit 6fabd03 into vmware:master Nov 29, 2019
@Didainius Didainius deleted the issue-323 branch November 29, 2019 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants