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 default_security_profile attribute to project #1374

Merged

Conversation

ksamoray
Copy link
Collaborator

Add the attribute above and manage it vs the VpcSecurityProfiles client

@ksamoray
Copy link
Collaborator Author

/test-all

if err != nil {
return err
}
return patchVpcSecurityProfile(d, connector, id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest to wrap this in HasChange ("default_security_profile") to avoid unnecessary call

_, err := client.Get(defaultOrgID, projectID, "default")

if isNotFoundError(err) && !enabled {
// No action required
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we dealing with the case in which default profile was not created yet? In this case, should we busy wait here?

@ksamoray
Copy link
Collaborator Author

/test-all

Add the attribute above and manage it vs the VpcSecurityProfiles client

Signed-off-by: Kobi Samoray <[email protected]>
@ksamoray
Copy link
Collaborator Author

/test-all

Copy link
Collaborator

@annakhm annakhm left a comment

Choose a reason for hiding this comment

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

We could use H-API to avoid two roundtrips but I don't think is worth it

@ksamoray ksamoray merged commit a7cb240 into vmware:vpc20-feature-branch Sep 22, 2024
5 checks passed
@ksamoray ksamoray deleted the project-default-sec-profile branch September 22, 2024 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants