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 extra nil check on ServeStale.DisableStaleWhileUpdating #4814

Merged

Conversation

guineveresaenger
Copy link
Contributor

@guineveresaenger guineveresaenger commented Dec 20, 2024

Hi there!

It prevents a panic on a nil value in the ServeStale.DisableStaleWhileUpdating field.

A repro is in the linked issue.

I was not able to run tests locally so I'm hoping to get a hand with adding a test case for this scenario. I'm also happy for a maintainer to take it forward from here. I've verified with a local build that this fixes the panic.

Fixes #4813

Copy link
Contributor

github-actions bot commented Dec 20, 2024

changelog detected ✅

@jacobbednarz
Copy link
Member

acceptance tests all passing

$ TF_ACC=1 go test ./internal/framework/service/rulesets/ -run "^TestAcc" -count 1 -v
=== RUN   TestAccCloudflareRuleset_WAFBasic
--- PASS: TestAccCloudflareRuleset_WAFBasic (4.53s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRuleset
--- PASS: TestAccCloudflareRuleset_WAFManagedRuleset (3.93s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetWithoutDescription
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetWithoutDescription (3.65s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetOWASP
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetOWASP (3.64s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetOWASPBlockXSSWithAnomalyOver60
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetOWASPBlockXSSWithAnomalyOver60 (4.15s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetOWASPOnlyPL1
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetOWASPOnlyPL1 (3.91s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetDeployMultiple
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetDeployMultiple (4.86s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithSkip
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithSkip (4.92s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithTopSkipAndLastSkip
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetDeployMultipleWithTopSkipAndLastSkip (4.89s)
=== RUN   TestAccCloudflareRuleset_SkipPhaseAndProducts
--- PASS: TestAccCloudflareRuleset_SkipPhaseAndProducts (4.00s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetWithCategoryAndRuleBasedOverrides
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetWithCategoryAndRuleBasedOverrides (4.09s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetWithIDBasedOverrides
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetWithIDBasedOverrides (3.53s)
=== RUN   TestAccCloudflareRuleset_MagicTransitUpdateWithHigherPriority
    acctest.go:112: Skipping acceptance test for default account (f037e56e89293a057740de681ac9abbe). Default account is not configured for Magic Transit.
--- SKIP: TestAccCloudflareRuleset_MagicTransitUpdateWithHigherPriority (0.00s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetWithPayloadLogging
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetWithPayloadLogging (3.67s)
=== RUN   TestAccCloudflareRuleset_RateLimit
--- PASS: TestAccCloudflareRuleset_RateLimit (4.20s)
=== RUN   TestAccCloudflareRuleset_RateLimitScorePerPeriod
--- PASS: TestAccCloudflareRuleset_RateLimitScorePerPeriod (3.50s)
=== RUN   TestAccCloudflareRuleset_RateLimitMitigationTimeoutOfZero
=== PAUSE TestAccCloudflareRuleset_RateLimitMitigationTimeoutOfZero
=== RUN   TestAccCloudflareRuleset_CustomErrors
--- PASS: TestAccCloudflareRuleset_CustomErrors (3.50s)
=== RUN   TestAccCloudflareRuleset_RequestOrigin
--- PASS: TestAccCloudflareRuleset_RequestOrigin (3.58s)
=== RUN   TestAccCloudflareRuleset_RequestOriginPortWithoutHost
--- PASS: TestAccCloudflareRuleset_RequestOriginPortWithoutHost (3.81s)
=== RUN   TestAccCloudflareRuleset_TransformationRuleURIPath
--- PASS: TestAccCloudflareRuleset_TransformationRuleURIPath (3.73s)
=== RUN   TestAccCloudflareRuleset_TransformationRuleURIQuery
--- PASS: TestAccCloudflareRuleset_TransformationRuleURIQuery (3.62s)
=== RUN   TestAccCloudflareRuleset_TransformationRuleURIPathAndQueryCombination
--- PASS: TestAccCloudflareRuleset_TransformationRuleURIPathAndQueryCombination (7.63s)
=== RUN   TestAccCloudflareRuleset_TransformationRuleRequestHeaders
--- PASS: TestAccCloudflareRuleset_TransformationRuleRequestHeaders (6.13s)
=== RUN   TestAccCloudflareRuleset_TransformationRuleResponseHeaders
--- PASS: TestAccCloudflareRuleset_TransformationRuleResponseHeaders (3.28s)
=== RUN   TestAccCloudflareRuleset_ResponseCompression
--- PASS: TestAccCloudflareRuleset_ResponseCompression (3.39s)
=== RUN   TestAccCloudflareRuleset_ActionParametersMultipleSkips
--- PASS: TestAccCloudflareRuleset_ActionParametersMultipleSkips (4.38s)
=== RUN   TestAccCloudflareRuleset_ActionParametersOverridesAction
--- PASS: TestAccCloudflareRuleset_ActionParametersOverridesAction (3.81s)
=== RUN   TestAccCloudflareRuleset_ActionParametersHTTPDDoSOverride
--- PASS: TestAccCloudflareRuleset_ActionParametersHTTPDDoSOverride (3.63s)
=== RUN   TestAccCloudflareRuleset_ActionParametersOverrideAllRulesetRules
--- PASS: TestAccCloudflareRuleset_ActionParametersOverrideAllRulesetRules (3.38s)
=== RUN   TestAccCloudflareRuleset_AccountLevelCustomWAFRule
--- PASS: TestAccCloudflareRuleset_AccountLevelCustomWAFRule (5.27s)
=== RUN   TestAccCloudflareRuleset_ExposedCredentialCheck
--- PASS: TestAccCloudflareRuleset_ExposedCredentialCheck (2.90s)
=== RUN   TestAccCloudflareRuleset_Logging
--- PASS: TestAccCloudflareRuleset_Logging (3.14s)
=== RUN   TestAccCloudflareRuleset_ConditionallySetActionParameterVersion
--- PASS: TestAccCloudflareRuleset_ConditionallySetActionParameterVersion (6.60s)
=== RUN   TestAccCloudflareRuleset_WAFManagedRulesetWithActionManagedChallenge
--- PASS: TestAccCloudflareRuleset_WAFManagedRulesetWithActionManagedChallenge (8.21s)
=== RUN   TestAccCloudflareRuleset_LogCustomField
--- PASS: TestAccCloudflareRuleset_LogCustomField (3.61s)
=== RUN   TestAccCloudflareRuleset_ActionParametersOverridesThrashingStatus
--- PASS: TestAccCloudflareRuleset_ActionParametersOverridesThrashingStatus (20.15s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsAllEnabled
--- PASS: TestAccCloudflareRuleset_CacheSettingsAllEnabled (4.36s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsOptionalsEmpty
--- PASS: TestAccCloudflareRuleset_CacheSettingsOptionalsEmpty (3.75s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsCacheReserveFalse
--- PASS: TestAccCloudflareRuleset_CacheSettingsCacheReserveFalse (3.53s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsOnlyExludeOrigin
--- PASS: TestAccCloudflareRuleset_CacheSettingsOnlyExludeOrigin (3.63s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsEdgeTTLRespectOrigin
--- PASS: TestAccCloudflareRuleset_CacheSettingsEdgeTTLRespectOrigin (3.79s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsNoCacheForStatus
--- PASS: TestAccCloudflareRuleset_CacheSettingsNoCacheForStatus (3.72s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsStatusRangeGreaterThan
--- PASS: TestAccCloudflareRuleset_CacheSettingsStatusRangeGreaterThan (3.93s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsStatusRangeLessThan
--- PASS: TestAccCloudflareRuleset_CacheSettingsStatusRangeLessThan (3.80s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsFalse
--- PASS: TestAccCloudflareRuleset_CacheSettingsFalse (3.58s)
=== RUN   TestAccCloudflareRuleset_Config
--- PASS: TestAccCloudflareRuleset_Config (3.49s)
=== RUN   TestAccCloudflareRuleset_Redirect
--- PASS: TestAccCloudflareRuleset_Redirect (5.09s)
=== RUN   TestAccCloudflareRuleset_DynamicRedirect
--- PASS: TestAccCloudflareRuleset_DynamicRedirect (3.92s)
=== RUN   TestAccCloudflareRuleset_DynamicRedirectWithoutPreservingQueryString
--- PASS: TestAccCloudflareRuleset_DynamicRedirectWithoutPreservingQueryString (4.09s)
=== RUN   TestAccCloudflareRuleset_TransformationRuleURIStripOffQueryString
--- PASS: TestAccCloudflareRuleset_TransformationRuleURIStripOffQueryString (3.47s)
=== RUN   TestAccCloudflareRuleset_TransformationRuleURIStripOffPath
--- PASS: TestAccCloudflareRuleset_TransformationRuleURIStripOffPath (4.14s)
=== RUN   TestAccCloudflareRuleset_ConfigSingleFalseyValue
--- PASS: TestAccCloudflareRuleset_ConfigSingleFalseyValue (3.61s)
=== RUN   TestAccCloudflareRuleset_ConfigConflictingCacheByDevice
--- PASS: TestAccCloudflareRuleset_ConfigConflictingCacheByDevice (0.23s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsMissingEdgeTTLWithOverrideOrigin
--- PASS: TestAccCloudflareRuleset_CacheSettingsMissingEdgeTTLWithOverrideOrigin (0.26s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsMissingBrowserTTLWithOverrideOrigin
--- PASS: TestAccCloudflareRuleset_CacheSettingsMissingBrowserTTLWithOverrideOrigin (0.17s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsInvalidEdgeTTLWithOverrideOrigin
--- PASS: TestAccCloudflareRuleset_CacheSettingsInvalidEdgeTTLWithOverrideOrigin (0.17s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsEdgeTTLWithBypassByDefault
--- PASS: TestAccCloudflareRuleset_CacheSettingsEdgeTTLWithBypassByDefault (3.46s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsInvalidEdgeTTLWithBypassByDefault
--- PASS: TestAccCloudflareRuleset_CacheSettingsInvalidEdgeTTLWithBypassByDefault (0.22s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsBrowserTTLWithBypass
--- PASS: TestAccCloudflareRuleset_CacheSettingsBrowserTTLWithBypass (3.55s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsInvalidBrowserTTLWithBypass
--- PASS: TestAccCloudflareRuleset_CacheSettingsInvalidBrowserTTLWithBypass (0.23s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsInvalidBrowserTTLWithOverrideOrigin
--- PASS: TestAccCloudflareRuleset_CacheSettingsInvalidBrowserTTLWithOverrideOrigin (0.17s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsDefinedQueryStringExcludeKeys
--- PASS: TestAccCloudflareRuleset_CacheSettingsDefinedQueryStringExcludeKeys (3.63s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsDefinedQueryStringIncludeKeys
--- PASS: TestAccCloudflareRuleset_CacheSettingsDefinedQueryStringIncludeKeys (3.78s)
=== RUN   TestAccCloudflareRuleset_CacheSettingsHandleDefaultHeaderExcludeOrigin
--- PASS: TestAccCloudflareRuleset_CacheSettingsHandleDefaultHeaderExcludeOrigin (8.52s)
=== RUN   TestAccCloudflareRuleset_ImportHandlesMissingValues
--- PASS: TestAccCloudflareRuleset_ImportHandlesMissingValues (0.29s)
=== CONT  TestAccCloudflareRuleset_RateLimitMitigationTimeoutOfZero
--- PASS: TestAccCloudflareRuleset_RateLimitMitigationTimeoutOfZero (4.01s)
PASS
ok  	github.com/cloudflare/terraform-provider-cloudflare/internal/framework/service/rulesets	257.341s

@jacobbednarz jacobbednarz merged commit b45c0b9 into cloudflare:master Dec 26, 2024
7 checks passed
@github-actions github-actions bot added this to the v4.49.0 milestone Dec 26, 2024
Copy link
Contributor

This functionality has been released in v4.49.0 of the Terraform Cloudflare 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 github-actions bot locked as resolved and limited conversation to collaborators Dec 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cloudflare_ruleset: Panic on import when serve_stale action parameter is empty/nil
2 participants