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

feat: network policy v1 readiness #2914

Merged
merged 10 commits into from
Jul 23, 2024
Merged

Conversation

sfc-gh-jcieslak
Copy link
Collaborator

Changes:

  • refactored the resource and created the data source for network policy
  • changes documented in the migration guide
  • examples refreshed/added

Test Plan

  • acceptance tests

References

@sfc-gh-jcieslak sfc-gh-jcieslak force-pushed the network-policy-v1-readiness branch from 69f7492 to 9c87035 Compare July 8, 2024 13:33
Copy link

github-actions bot commented Jul 8, 2024

Integration tests failure for 69f749211c6fd9c16a9f594b91eaa62264dc4453

Copy link

github-actions bot commented Jul 8, 2024

Integration tests failure for 9c870354a2ae06c8f13ad89d57067eb2a9db76d0

pkg/sdk/network_policies_gen.go Show resolved Hide resolved
pkg/datasources/network_policies_acceptance_test.go Outdated Show resolved Hide resolved
pkg/acceptance/helpers/network_rule_client.go Outdated Show resolved Hide resolved
pkg/resources/network_policy.go Show resolved Hide resolved
pkg/resources/network_policy.go Outdated Show resolved Hide resolved
pkg/resources/network_policy.go Outdated Show resolved Hide resolved
pkg/acceptance/helpers/network_rule_client.go Outdated Show resolved Hide resolved
pkg/datasources/network_policies_acceptance_test.go Outdated Show resolved Hide resolved
pkg/datasources/network_policies_acceptance_test.go Outdated Show resolved Hide resolved
pkg/datasources/network_policies_acceptance_test.go Outdated Show resolved Hide resolved
pkg/resources/network_policy.go Show resolved Hide resolved
pkg/sdk/network_policies_gen.go Show resolved Hide resolved
Copy link

Integration tests failure for e9bce1568e0e7c2c110b319a6446be4aba9fd71c

Copy link

Integration tests failure for 59fbe5a16ffe67cbcc9cab159f3156bffbaad05e

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review July 12, 2024 15:26
pkg/resources/diff_suppressions.go Outdated Show resolved Hide resolved
pkg/resources/diff_suppressions_test.go Outdated Show resolved Hide resolved
pkg/resources/diff_suppressions_test.go Outdated Show resolved Hide resolved
pkg/resources/diff_suppressions_test.go Outdated Show resolved Hide resolved
pkg/resources/network_policy.go Show resolved Hide resolved
MIGRATION_GUIDE.md Show resolved Hide resolved
pkg/helpers/helpers.go Outdated Show resolved Hide resolved
pkg/helpers/helpers_test.go Outdated Show resolved Hide resolved
@@ -92,18 +92,22 @@ func NetworkPolicy() *schema.Resource {
Description: "Resource used to control network traffic. For more information, check an [official guide](https://docs.snowflake.com/en/user-guide/network-policies) on controlling network traffic with network policies.",

CustomizeDiff: customdiff.All(
// For now, allowed_network_rule_list and blocked_network_rule_list have to stay commented and the implementation
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmmm, I think that I tested it somewhere, and even had to adjust the implementation because of that, that the order of suppressions is suppress on field -> customized diff.

Are you sure that this is not something else causing the problems here? With that line of argumentation, all the diff suppressed attributes should be problematic (e.g. all the enums where me allow lowercas for example - there are tests in warehouse for it).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried, but couldn't find anything that would make sense as to why Sets are behaving that way, but I believe that may be strictly connected to how the diff suppressions are done in the old TF SDK. I checked that before the custom diff call all of the diff suppressions were returned with true. That means it should suppress the whole object change, and it does, but it's not reflected in the custom diff which seems to take different things into consideration when calling d.HasChanges()...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, please change this comment and document the findings - now, the comment is misleading, because this is not the problem with diff suppression, but the problem in how in the SDK v2 diff suppression is handled for the subset of params (lists, sets, maps).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

👌 (Will be addressed in the next pr)

# Conflicts:
#	MIGRATION_GUIDE.md
#	pkg/acceptance/helpers/network_rule_client.go
#	pkg/acceptance/helpers/test_client.go
#	pkg/helpers/helpers.go
#	pkg/helpers/helpers_test.go
Copy link

Integration tests failure for 40290ec7cc89c3cb4ee4a95078ab9308bca0fe17

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review July 23, 2024 09:45
@sfc-gh-jcieslak sfc-gh-jcieslak merged commit 3408c3f into main Jul 23, 2024
8 of 9 checks passed
@sfc-gh-jcieslak sfc-gh-jcieslak deleted the network-policy-v1-readiness branch July 23, 2024 11:06
sfc-gh-jcieslak pushed a commit that referenced this pull request Jul 26, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.94.0](v0.93.0...v0.94.0)
(2024-07-26)


### 🎉 **What's new:**

* Add missing session parameters
([#2936](#2936))
([4ce662d](4ce662d))
* Adjust user SDK
([#2947](#2947))
([1127bb3](1127bb3))
* Better tests poc
([#2917](#2917))
([ef496c2](ef496c2))
* Introduce assertions generators part1
([#2952](#2952))
([1582a9f](1582a9f))
* Introduce assertions generators part2
([#2956](#2956))
([f715e8a](f715e8a))
* network policy v1 readiness
([#2914](#2914))
([3408c3f](3408c3f))
* Rework schema datasource
([#2954](#2954))
([f70e40e](f70e40e))
* Rework schema resource
([#2955](#2955))
([400a5c8](400a5c8))
* Role v1 readiness
([#2916](#2916))
([32c7690](32c7690))
* Schema SDK upgrade
([#2945](#2945))
([bca0836](bca0836))
* Streamlit v1 readiness
([#2930](#2930))
([aa42260](aa42260))


### 🔧 **Misc**

* Remove deprecation from unsafe execute
([#2941](#2941))
([ed712d7](ed712d7))
* Update documentation
([#2931](#2931))
([da98bc3](da98bc3))


### 🐛 **Bug fixes:**

* ATTRIBUTE set(string) parsing for cortex search service
([#2953](#2953))
([70a1c9a](70a1c9a))
* external function header parsing and add missing privileges
([#2961](#2961))
([9d882fe](9d882fe))
* Fix sync_password field for Azure scim clients
([#2950](#2950))
([6781133](6781133))
* Fix tests and relax warehouse validations
([#2959](#2959))
([dd01ce9](dd01ce9)),
closes
[#2948](#2948)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
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.

3 participants