Repository: cloud-custodian/cloud-custodian

Issue:
filter using value_from takes excessively long
**Describe the bug**
We have a security-group ingress filter that uses `value_from` to pull a list of cidr values from a url.  This causes the filtering step to take roughly 15 seconds to filter 34 security groups.  If I replace this with a `value` with a list of cidr addresses, the filter step is done in milliseconds.

**To Reproduce**
The following security-group filter:
```yaml
  - name: hourly-aws-std-sg01-compliance-scan
    resource: security-group
    filters:
      - type: ingress
        Cidr:
          op: in
          value_from:
            url: https://api.cloudflare.com/client/v4/ips
            format: json
            expr: "[result.ipv4_cidrs, result.ipv6_cidrs]|[]"
```
produced the following timing (17.5 seconds to filter 34 security-groups)
```
2020-04-02 14:41:49,574: custodian.resources.securitygroup:DEBUG Using cached c7n.resources.vpc.SecurityGroup: 34
2020-04-02 14:42:07,069: custodian.resources.securitygroup:DEBUG Filtered from 34 to 1 securitygroup
2020-04-02 14:42:07,069: custodian.policy:INFO policy:hourly-aws-std-sg01-compliance-scan resource:security-group region:us-west-2 count:1 time:17.50
```

**Expected behavior**
If I replace the filter with a static list of cidrs to compare against:
```yaml
  - name: hourly-aws-std-sg01-compliance-scan
    resource: security-group
    filters:
      - type: ingress
        Cidr:
          op: in
          value:
            - "173.245.48.0/20"
            - "103.21.244.0/22"
            - "103.22.200.0/22"
            - "103.31.4.0/22"
            - "141.101.64.0/18"
            - "108.162.192.0/18"
            - "190.93.240.0/20"
            - "188.114.96.0/20"
            - "197.234.240.0/22"
            - "198.41.128.0/17"
            - "162.158.0.0/15"
            - "104.16.0.0/12"
            - "172.64.0.0/13"
            - "131.0.72.0/22"
            - "2400:cb00::/32"
            - "2606:4700::/32"
            - "2803:f800::/32"
            - "2405:b500::/32"
            - "2405:8100::/32"
            - "2a06:98c0::/29"
            - "2c0f:f248::/32"
```
it's nearly instantaneous:
```
2020-04-02 14:45:27,569: custodian.resources.securitygroup:DEBUG Using cached c7n.resources.vpc.SecurityGroup: 34
2020-04-02 14:45:27,570: custodian.resources.securitygroup:DEBUG Filtered from 34 to 1 securitygroup
2020-04-02 14:45:27,570: custodian.policy:INFO policy:hourly-aws-std-sg01-compliance-scan resource:security-group region:us-west-2 count:1 time:0.00
```

**Background (please complete the following information):**
 - OS: OSX 10.15.4
 - Python Version: python 3.7.6
 - Custodian Version: 0.8.46.1

Discussion:
JohnHillegass: > this closes #5547 as we were saving the results in the resolver to a cache, but never looking them up. s3 was being cached because of the generic boto session caching that happens.

Thanks for submitting, I added a few comments but besides those, looks good!
tjstansell: @kapilt would you mind taking this to address the code coverage ci check?  i'm not sure exactly what kind of testing changes would be required to fix this.
kapilt: Thanks for the pull request! Looking over this it seems we shouldn't be caching in resolver, but instead in values from, as that will save on the deserialization/expression logic, albeit trade off on a fetch per unique expression/url combo across policies, but for embedded value filter scenarios like this that should be a significant speed up.
tjstansell: Or both.  cache in resolver for raw url queries ... but also cache at the value from layer?  then you're only fetching the url once no matter how many combinations of filter expressions you might have...

Patch:
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index ffb2b7e31..07eb3a816 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -3,150 +3,26 @@ trigger:
 
 variables:
 - group: CustodianCoreCI
-- name: WINDOWS_CACHE_DIR
-  value: $(LOCALAPPDATA)\pip\cache
 
 jobs:
 
-- job: 'Lint'
-  pool:
-    vmImage: 'Ubuntu-18.04'
-  steps:
-    - checkout: self
-      fetchDepth: 1
-
-    - task: UsePythonVersion@0
-      inputs:
-        versionSpec: '3.7'
-        architecture: 'x64'
-
-    - script: python -m pip install --upgrade pip && pip install flake8
-      displayName: "Install Dependencies"
-
-    - script: make lint
-      displayName: "Flake8 Lint"
-
-
-- job: 'Docs'
-  pool:
-    vmImage: 'Ubuntu-18.04'
-  steps:
-  - checkout: self
-    fetchDepth: 1
-
-  - task: UsePythonVersion@0
-    inputs:
-      versionSpec: '3.7'
-      architecture: 'x64'
-
-  - script: python -m pip install --upgrade pip && pip install tox
-    displayName: 'Install Test Runner'
-
-  - script: TOXENV=docs tox --notest
-    displayName: 'Install Dependencies'
-
-  - script: TOXENV=docs tox
-    displayName: "Docs Build"
-
-
-- job: 'Test'
+- job: 'CustodianCask'
+  displayName: 'Tool Tests - Cask'
   pool:
     vmImage: 'Ubuntu-18.04'
-  strategy:
-    matrix:
-      Python36:
-        python.version: '3.6'
-        TOXENV: py36-cov
-    maxParallel: 4
-
-  steps:
-  - checkout: self
-    fetchDepth: 2
-
-  - task: UsePythonVersion@0
-    displayName: "Set Python Version"
-    inputs:
-      versionSpec: '$(python.version)'
-      architecture: 'x64'
-
-  - script: python -m pip install --upgrade pip && pip install tox codecov
-    displayName: 'Install Test Runner'
-
-  - script: tox --notest
-    displayName: 'Install Dependencies'
-
-  - script: tox
-    displayName: 'Tox'
-
-  - script: |
-      ./.tox/$TOXENV/bin/coverage xml && \
-      bash <(curl -s https://codecov.io/bash) -Z \
-       -X gcov -X coveragepy -X search -X xcode -X gcovout -X fix \
-       -f coverage.xml
-    displayName: 'Publish Code Coverage'
-    condition: "eq(variables['python.version'], '3.6')"
-    env:
-      CODECOV_TOKEN: $(CODECOV_TOKEN)
-
-  - task: PublishTestResults@2
-    displayName: "Publish Test Results Azure"
-    inputs:
-      testResultsFiles: '**/test-results.xml'
-      testRunTitle: 'Python $(python.version)'
-    condition: succeededOrFailed()
-
-- job: 'TestWindows'
-  displayName: 'Test Win2019'
-  pool:
-    vmImage: 'windows-2019'
-  strategy:
-    matrix:
-      Python36:
-        python.version: '3.6'
-        TOXENV: py36
-      Python37:
-        python.version: '3.7'
-        TOXENV: py37-windows
-    maxParallel: 3
 
   steps:
-  - checkout: self
-    fetchDepth: 2
-
-  - powershell: |
-      $date=$(Get-Date).DayOfYear;
-      Write-Host "##vso[task.setvariable variable=DAY_OF_YEAR]$date"
-    displayName:  Set cache date key
-
-  - task: CacheBeta@0
-    inputs:
-      key: '"c7n-tox" | "$(python.version)" | "$(Agent.OS)" | "$(DAY_OF_YEAR)"'
-      path: $(WINDOWS_CACHE_DIR)
-
-  - task: UsePythonVersion@0
-    displayName: "Set Python Version"
+  - task: GoTool@0
     inputs:
-      versionSpec: '$(python.version)'
-      architecture: 'x64'
-
-  - script: python -m pip install --upgrade pip && pip install tox
-    displayName: 'Install Test Runner'
+      version: '1.12.6'
 
-  - script: tox --notest
-    displayName: 'Install Dependencies'
+  - script: make test
+    workingDirectory: tools/cask/
+    displayName: 'Build for all platforms'
 
-  - script: tox
-    displayName: 'Tox'
 
-  - task: PublishTestResults@2
-    displayName: "Publish Test Results Azure"
-    inputs:
-      testResultsFiles: '**/test-results.xml'
-      testRunTitle: 'Python $(python.version) Windows'
-    condition: succeededOrFailed()
-
-- job: 'CustodianCask'
-  displayName: 'Container Cask'
+- job: 'CustodianOmniSSM'
+  displayName: 'Tool Tests - OmniSSM'
   pool:
     vmImage: 'Ubuntu-18.04'
 
@@ -154,7 +30,6 @@ jobs:
   - task: GoTool@0
     inputs:
       version: '1.12.6'
-
-  - script: make release
-    workingDirectory: tools/cask/
-    displayName: 'Build for all platforms'
+  - script: make test
+    workingDirectory: tools/omnissm/
+    displayName: 'Test'
diff --git a/.github/workflows/ci-master.yml b/.github/workflows/ci-master.yml
index c6d4e0ef4..e2696f10e 100644
--- a/.github/workflows/ci-master.yml
+++ b/.github/workflows/ci-master.yml
@@ -9,12 +9,32 @@ on:
       - master
 
 jobs:
+  Lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v1
+        with:
+          python-version: 3.8
+      - name: Install Linter
+        run: |
+          python -m pip install --upgrade pip
+          pip install flake8
+      - name: Lint Check
+        run: |
+          make lint
+
   Tests:
     runs-on: "${{ matrix.os }}"
+    needs: Lint
     strategy:
       matrix:
         os: [macos-latest, ubuntu-latest]
         python-version: [3.6, 3.7, 3.8]
+        exclude:
+          # conserve some osx builders
+          - python-version: 3.6
+            os: macos-latest
         include:
           - python-version: 3.6
             os: ubuntu-latest
@@ -24,10 +44,7 @@ jobs:
             tox-target: py37
           - python-version: 3.8
             os: ubuntu-latest
-            tox-target: py38
-          - python-version: 3.6
-            os: macos-latest
-            tox-target: py36
+            tox-target: py38-cov
           - python-version: 3.7
             tox-target: py37
             os: macos-latest
@@ -45,16 +62,73 @@ jobs:
         with:
           path: .tox/${{ matrix.tox-target }}
           key: venv-${{ runner.os }}-${{ matrix.python-version }}-${{ hashFiles('**/requirement*.txt') }}
+      - name: Install Test Runner
+        run: |
+          python -m pip install --upgrade pip
+          pip install tox
+      - name: Install Deps
+        run: |
+          tox -e ${{ matrix.tox-target }} --notest
+      - name: Test
+        run: |
+          tox -vv -e ${{ matrix.tox-target }}
+
+      - name: Coverage Generate XML
+        if: contains(matrix.tox-target, 'py38-cov')
+        run: |
+          ./.tox/${{ matrix.tox-target }}/bin/coverage xml && \
+          bash <(curl -s https://codecov.io/bash) -Z \
+          -f coverage.xml
 
+  Test-Windows:
+    # windows can't use the fast cache technique we use in our matrix builds
+    # where we cache the entire tox virtualenv directory, without error.
+    # instead we just cache the pip downloads, and install every time.
+
+    runs-on: windows-latest
+    needs: Lint
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v1
+        with:
+          python-version: 3.8
+      - name: Set up cache
+        uses: actions/cache@v1
+        with:
+          path: ~\AppData\Local\pip\Cache
+          key: pip-${{ 3.8 }}-${{ hashFiles('**/requirement*.txt') }}
       - name: Install Test Runner
         run: |
           python -m pip install pip
           pip install tox codecov
-
       - name: Install Deps
         run: |
-          tox -e ${{ matrix.tox-target }} --notest
-
+          tox -e py38 --notest
       - name: Test
         run: |
-          tox -e ${{ matrix.tox-target }}
+          tox -e py38
+
+  Docs:
+    # todo, figure out how to fast cache the tox directory here.
+    runs-on: ubuntu-latest
+    needs: Lint
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-python@v1
+        with:
+          python-version: 3.8
+      - name: Set up cache
+        uses: actions/cache@v1
+        with:
+          path: "~/.cache/pip"
+          key: docs-${{ runner.os }}-${{ 3.8 }}-${{ hashFiles('**/requirement*.txt') }}
+      - name: Install Tox
+        run: |
+          python -m pip install --upgrade pip
+          pip install tox
+      - name: Install Build Deps
+        run: |
+          TOXENV=docs tox --notest
+      - name: Build Docs
+        run: |
+          TOXENV=docs tox
diff --git a/README.md b/README.md
index ed1adf46c..2a8773e3d 100644
--- a/README.md
+++ b/README.md
@@ -6,7 +6,7 @@ Cloud Custodian
 ---
 
 [![](https://badges.gitter.im/cloud-custodian/cloud-custodian.svg)](https://gitter.im/cloud-custodian/cloud-custodian?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
-![CI](https://github.com/cloud-custodian/cloud-custodian/workflows/CI/badge.svg?event=push)
+[![CI](https://github.com/cloud-custodian/cloud-custodian/workflows/CI/badge.svg?event=push)](https://github.com/cloud-custodian/cloud-custodian/actions?query=workflow%3ACI+branch%3Amaster+event%3Apush)
 [![](https://dev.azure.com/cloud-custodian/cloud-custodian/_apis/build/status/Custodian%20-%20CI?branchName=master)](https://dev.azure.com/cloud-custodian/cloud-custodian/_build)
 [![](https://img.shields.io/badge/license-Apache%202-blue.svg)](https://www.apache.org/licenses/LICENSE-2.0)
 [![](https://codecov.io/gh/cloud-custodian/cloud-custodian/branch/master/graph/badge.svg)](https://codecov.io/gh/cloud-custodian/cloud-custodian)
diff --git a/c7n/resolver.py b/c7n/resolver.py
index 2b4ad4f35..3cc37e6ba 100644
--- a/c7n/resolver.py
+++ b/c7n/resolver.py
@@ -37,6 +37,11 @@ class URIResolver:
         self.cache = cache
 
     def resolve(self, uri):
+        if self.cache:
+            contents = self.cache.get(("uri-resolver", uri))
+            if contents is not None:
+                return contents
+
         if uri.startswith('s3://'):
             contents = self.get_s3_uri(uri)
         else:
@@ -46,7 +51,8 @@ class URIResolver:
             with closing(urlopen(req)) as response:
                 contents = self.handle_response_encoding(response)
 
-        self.cache.save(("uri-resolver", uri), contents)
+        if self.cache:
+            self.cache.save(("uri-resolver", uri), contents)
         return contents
 
     def handle_response_encoding(self, response):
@@ -93,7 +99,7 @@ class ValuesFrom:
          url: s3://bucket/xyz/foo.json
          expr: [].AppId
 
-      values_from:
+      value_from:
          url: http://foobar.com/mydata
          format: json
          expr: Region."us-east-1"[].ImageId
@@ -129,6 +135,7 @@ class ValuesFrom:
         }
         self.data = format_string_values(data, **config_args)
         self.manager = manager
+        self.cache = manager._cache
         self.resolver = URIResolver(manager.session_factory, manager._cache)
 
     def get_contents(self):
@@ -147,6 +154,21 @@ class ValuesFrom:
         return contents, format
 
     def get_values(self):
+        if self.cache:
+            # use these values as a key to cache the result so if we have
+            # the same filter happening across many resources, we can reuse
+            # the results.
+            key = [self.data.get(i) for i in ('url', 'format', 'expr')]
+            contents = self.cache.get(("value-from", key))
+            if contents is not None:
+                return contents
+
+        contents = self._get_values()
+        if self.cache:
+            self.cache.save(("value-from", key), contents)
+        return contents
+
+    def _get_values(self):
         contents, format = self.get_contents()
 
         if format == 'json':
diff --git a/c7n/resources/s3.py b/c7n/resources/s3.py
index 77b0b21f5..840b43bc2 100644
--- a/c7n/resources/s3.py
+++ b/c7n/resources/s3.py
@@ -1414,7 +1414,7 @@ class FilterPublicBlock(Filter):
 
     def process_bucket(self, bucket):
         s3 = bucket_client(local_session(self.manager.session_factory), bucket)
-        config = {key: False for key in self.keys}
+        config = dict(bucket.get(self.annotation_key, {key: False for key in self.keys}))
         if self.annotation_key not in bucket:
             try:
                 config = s3.get_public_access_block(
@@ -1437,27 +1437,50 @@ class FilterPublicBlock(Filter):
 class SetPublicBlock(BucketActionBase):
     """Action to update Public Access blocks on S3 buckets
 
-    if no action parameters are provided it will enable all public block configuration.
+    If no action parameters are provided all settings will be set to the `state`, which defaults
 
-    If a action parameters are provided, those will be set and other extant values preserved.
+    If action parameters are provided, those will be set and other extant values preserved.
 
     :example:
 
     .. code-block:: yaml
 
             policies:
-              - name: s3-dont-ignore-public-acls
+              - name: s3-public-block-enable-all
                 resource: s3
                 filters:
                   - type: check-public-block
                 actions:
                   - type: set-public-block
-                  # BlockPublicAcls: true <-- optional, if none provided sets all to true by default
+
+            policies:
+              - name: s3-public-block-disable-all
+                resource: s3
+                filters:
+                  - type: check-public-block
+                actions:
+                  - type: set-public-block
+                    state: false
+
+            policies:
+              - name: s3-public-block-enable-some
+                resource: s3
+                filters:
+                  - or:
+                    - type: check-public-block
+                      BlockPublicAcls: false
+                    - type: check-public-block
+                      BlockPublicPolicy: false
+                actions:
+                  - type: set-public-block
+                    BlockPublicAcls: true
+                    BlockPublicPolicy: true
 
     """
 
     schema = type_schema(
         'set-public-block',
+        state={'type': 'boolean', 'default': True},
         BlockPublicAcls={'type': 'boolean'},
         IgnorePublicAcls={'type': 'boolean'},
         BlockPublicPolicy={'type': 'boolean'},
@@ -1489,7 +1512,7 @@ class SetPublicBlock(BucketActionBase):
                 config[key] = self.data.get(key)
         else:
             for key in self.keys:
-                config[key] = True
+                config[key] = self.data.get('state', True)
         s3.put_public_access_block(
             Bucket=bucket['Name'], PublicAccessBlockConfiguration=config)
 
diff --git a/requirements-dev.txt b/requirements-dev.txt
index a945a1224..1ac6d0612 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -1,6 +1,6 @@
 # Load all project requirements
 
-# note all of these are generated from poetry lock/projects via `make pkg-gen-requirements`
+# Note all of these are generated from poetry lock/projects via `make pkg-gen-requirements`
 -r requirements.txt
 -r tools/c7n_azure/requirements.txt
 -r tools/c7n_gcp/requirements.txt
diff --git a/tools/omnissm/Makefile b/tools/omnissm/Makefile
index afe11368c..ff10fdb47 100644
--- a/tools/omnissm/Makefile
+++ b/tools/omnissm/Makefile
@@ -8,7 +8,7 @@ functions := $(wildcard functions/*/.)
 build:
 	@mkdir -p bin
 	@go build -gcflags $(GCFLAGS) -ldflags $(LDFLAGS) -o bin/omnissm-agent ./cmd/omnissm-agent
-	
+
 build-linux:
 	@mkdir -p bin
 	@GOOS=linux GOARCH=amd64 go build -gcflags $(GCFLAGS) -ldflags $(LDFLAGS) -o bin/omnissm-agent-linux-amd64 ./cmd/omnissm-agent
diff --git a/tox.ini b/tox.ini
index e5db04705..fa8947a11 100644
--- a/tox.ini
+++ b/tox.ini
@@ -30,11 +30,16 @@ setenv =
     GOOGLE_APPLICATION_CREDENTIALS=tools/c7n_gcp/tests/data/credentials.json
 
 
-[testenv:py36]
+[testenv:py38]
 commands =
-    py.test -n {env:C7N_TEST_WORKERS:auto} tests tools {posargs}
+    pytest -n {env:C7N_TEST_WORKERS:auto} tests tools {posargs}
 
-[testenv:py38]
+[testenv:py38-cov]
+commands =
+    py.test -n {env:C7N_TEST_WORKERS:auto} --cov c7n --cov tools/c7n_azure/c7n_azure --cov tools/c7n_gcp/c7n_gcp --cov tools/c7n_kube/c7n_kube --cov tools/c7n_mailer/c7n_mailer tests tools {posargs}
+
+
+[testenv:py36]
 commands =
     py.test -n {env:C7N_TEST_WORKERS:auto} tests tools {posargs}
 
@@ -43,6 +48,8 @@ commands =
     py.test -n {env:C7N_TEST_WORKERS:auto} --cov c7n --cov tools/c7n_azure/c7n_azure --cov tools/c7n_gcp/c7n_gcp --cov tools/c7n_kube/c7n_kube --cov tools/c7n_mailer/c7n_mailer tests tools {posargs}
 
 [testenv:py37]
+;; override because this runner will run with schema validation on test policies
+;; by setting C7N_VALIDATE=true
 setenv =
     AWS_DEFAULT_REGION=us-east-1
     AWS_ACCESS_KEY_ID=foo
@@ -84,7 +91,7 @@ deps =
 [pytest]
 junit_family=xunit1
 ; for travis, we set this up as env var that we can override to limit concurrency
-addopts= --tb=native --durations 50 --junitxml=junit/test-results.xml
+addopts= --tb=native --durations 50
 markers =
   functional
   skiplive


Test patch:
diff --git a/tests/test_resolver.py b/tests/test_resolver.py
index 561373a8c..4786d4ab0 100644
--- a/tests/test_resolver.py
+++ b/tests/test_resolver.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 import csv
 import json
+import pickle
 import os
 import tempfile
 import vcr
@@ -30,12 +31,16 @@ class FakeCache:
 
     def __init__(self):
         self.state = {}
+        self.gets = 0
+        self.saves = 0
 
     def get(self, key):
-        return self.state.get(key)
+        self.gets += 1
+        return self.state.get(pickle.dumps(key))
 
     def save(self, key, data):
-        self.state[key] = data
+        self.saves += 1
+        self.state[pickle.dumps(key)] = data
 
 
 class FakeResolver:
@@ -72,7 +77,7 @@ class ResolverTest(BaseTest):
         uri = "s3://%s/resource.json?RequestPayer=requestor" % bname
         data = resolver.resolve(uri)
         self.assertEqual(content, data)
-        self.assertEqual(list(cache.state.keys()), [("uri-resolver", uri)])
+        self.assertEqual(list(cache.state.keys()), [pickle.dumps(("uri-resolver", uri))])
 
     def test_handle_content_encoding(self):
         session_factory = self.replay_flight_data("test_s3_resolver")
@@ -106,9 +111,9 @@ class UrlValueTest(BaseTest):
     def tearDown(self):
         os.chdir(self.old_dir)
 
-    def get_values_from(self, data, content):
+    def get_values_from(self, data, content, cache=None):
         config = Config.empty(account_id=ACCOUNT_ID)
-        mgr = Bag({"session_factory": None, "_cache": None, "config": config})
+        mgr = Bag({"session_factory": None, "_cache": cache, "config": config})
         values = ValuesFrom(data, mgr)
         values.resolver = FakeResolver(content)
         return values
@@ -181,3 +186,16 @@ class UrlValueTest(BaseTest):
         )
         self.assertEqual(values.get_values(), ["east-resource"])
         self.assertEqual(values.data.get("url", ""), ACCOUNT_ID)
+
+    def test_value_from_caching(self):
+        cache = FakeCache()
+        values = self.get_values_from(
+            {"url": "", "expr": '["{region}"][]', "format": "json"},
+            json.dumps({"us-east-1": "east-resource"}),
+            cache=cache,
+        )
+        self.assertEqual(values.get_values(), ["east-resource"])
+        self.assertEqual(values.get_values(), ["east-resource"])
+        self.assertEqual(values.get_values(), ["east-resource"])
+        self.assertEqual(cache.saves, 1)
+        self.assertEqual(cache.gets, 3)
diff --git a/tests/test_s3.py b/tests/test_s3.py
index e8a19b436..0ffc87507 100644
--- a/tests/test_s3.py
+++ b/tests/test_s3.py
@@ -1733,7 +1733,49 @@ class S3Test(BaseTest):
         self.assertEqual(len(resources), 1)
         self.assertEqual(resources[0]["Name"], bname)
 
-        # Make sure that the public blocks are enabled on the buckets
+        # Make sure that the public blocks are disabled on the buckets
+        response = client.get_public_access_block(
+            Bucket=bname)['PublicAccessBlockConfiguration']
+        for key in response.keys():
+            self.assertEqual(response[key], False)
+
+    def test_set_public_block_disable_all_via_state(self):
+        bname = 'mypublicblock'
+
+        self.patch(s3.S3, "executor_factory", MainThreadExecutor)
+        self.patch(s3, "S3_AUGMENT_TABLE", [])
+
+        session_factory = self.replay_flight_data("test_s3_public_block_disable_all")
+        session = session_factory()
+        client = session.client("s3")
+
+        p = self.load_policy(
+            {
+                "name": "CheckForPublicBlocks-Present",
+                "resource": "s3",
+                "filters": [
+                    {"Name": bname},
+                    {
+                        "type": "check-public-block",
+                        "BlockPublicAcls": True
+                    }
+                ],
+                "actions": [
+                    {
+                        "type": "set-public-block",
+                        "state": False
+                    }
+                ]
+            },
+            session_factory=session_factory,
+        )
+
+        # Test that there was a bucket with missing public blocks
+        resources = p.run()
+        self.assertEqual(len(resources), 1)
+        self.assertEqual(resources[0]["Name"], bname)
+
+        # Make sure that the public blocks are disabled on the buckets
         response = client.get_public_access_block(
             Bucket=bname)['PublicAccessBlockConfiguration']
         for key in response.keys():