GoogleCloudPlatform / deploymentmanager-samples

Deployment Manager samples and templates.
Apache License 2.0
946 stars 716 forks source link

Cloud Audit Logging #52

Closed davidebelloni closed 6 years ago

davidebelloni commented 7 years ago

Is there a way to set 'auditConfigs' in project resources? If I inject in 'gcpIamPolicy' (file project.py) the 'auditLogConfigs' json tree doesn't works. Like 'iam-policy' and 'gcpIamPolicy'.

Thanks

aljim commented 7 years ago

Is possible to call setIamPolicy method directly See https://github.com/GoogleCloudPlatform/deploymentmanager-samples/blob/master/examples/v2/project_creation/project.py#L99

the gcpIamPolicyPatch section is optional.

accessControl is limited to bindings

davidebelloni commented 7 years ago

Hi, I've to modify the "$(ref.get-iam-policy)" dictionary or use gcpIamPolicyPatch section?

davidebelloni commented 7 years ago

Ok, here what I've done:

    resources.extend([{
        'name': 'patch-auditConfigs-policy',
        'action': 'gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.setIamPolicy',
        'properties': {
          'resource': project_id,
          'policy': {'auditConfigs': [{
                      'auditLogConfigs': [{'logType':'DATA_READ'},{'logType':'ADMIN_READ'}],
                      'service': 'cloudkms.googleapis.com'}]},
          'updateMask': "auditConfigs,etag"
        }
    }])

Thanks for the support!

davidebelloni commented 7 years ago

Hi @aljim, I've tried to extend what this does:

  patch = []
  if context.properties.get('set-dm-service-account-as-owner'):
      patch.extend([{
        'role': 'roles/owner',
        'members': [
          'serviceAccount:$(ref.project.projectNumber)@cloudservices.gserviceaccount.com'
        ]
      }])
  if context.properties.get('set-default-iam'):
      patch.extend([{
        'role': 'roles/deploymentmanager.editor',
        'members': [
          'group:team-'+ project_id +'-iac@my.domain'
        ]
      },{
        'role': 'roles/deploymentmanager.viewer',
        'members': [
          'group:team-'+ project_id +'-admins@my.domain'
        ]
      }])
  if context.properties.get('set-ce-service-account-as-editor'):
      patch.extend([{
        'role': 'roles/editor',
        'members': [
          'serviceAccount:$(ref.project.projectNumber)-compute@developer.gserviceaccount.com'
        ]
      }])
  if context.properties.get('set-cr-service-account-as-editor'):
      patch.extend([{
        'role': 'roles/editor',
        'members': [
          'serviceAccount:service-$(ref.project.projectNumber)@containerregistry.iam.gserviceaccount.com'
        ]
      }])
  if context.properties.get('set-ke-service-account-as-service-agent'):
      patch.extend([{
        'role': 'roles/container.serviceAgent',
        'members': [
          'serviceAccount:service-$(ref.project.projectNumber)@container-engine-robot.iam.gserviceaccount.com'
        ]
      }])

  if patch:
    resources.extend([{
        'name': 'get-iam-policy',
        'action': 'gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.getIamPolicy',
        'properties': {
          'resource': project_id,
        },
        'metadata': {
          'dependsOn': [ApiResourceName(project_id, 'deploymentmanager.googleapis.com')]
        }
    }, {
        'name': 'patch-iam-policy',
        'action': 'gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.setIamPolicy',
        'properties': {
          'resource': project_id,
          'policy': '$(ref.get-iam-policy)',
          'gcpIamPolicyPatch': {
             'add': patch
           }
        }
    }])

This code has a strange behaviour and, without errors, doesn't seems to patch the actual IAM Policy

Thnaks

davidebelloni commented 6 years ago

HI @aljim @krwenholz , some news on the last comment?

Now I'm trying to use this conf also:

    resources.extend([{
        'name': 'patch-auditConfigs-policy',
        'action': 'gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.setIamPolicy',
        'properties': {
          'resource': project_id,
          'policy': {'auditConfigs': [
            {'auditLogConfigs': [{'logType':'DATA_READ'},{'logType':'ADMIN_READ'}], 'service': 'cloudkms.googleapis.com'},
            {'auditLogConfigs': [{'logType':'DATA_WRITE'},], 'service': 'cloudsql.googleapis.com'}
            ]},
          'updateMask': "auditConfigs,etag"
        }
    }])

but I've:

$ gcloud beta deployment-manager deployments update "$DMPROJECT" --config project.yaml
The fingerprint of the deployment is fXVaWhAcbaw3VM-pWMmyBA==
Waiting for update [operation-1510936931013-55e306b0d1189-f22e54b6-ab02daaa]...failed.                                                                                                                                                                                                                                    
ERROR: (gcloud.beta.deployment-manager.deployments.update) Error in Operation [operation-1510936931013-55e306b0d1189-f22e54b6-ab02daaa]: errors:
- code: COLLECTION_NOT_FOUND
  message: Collection 'cloudresourcemanager.projects.setIamPolicy' not found in discovery
    doc 'https://cloudresourcemanager.googleapis.com/$discovery/rest?version=v1'

If i return to this I've no issue:

    resources.extend([{
        'name': 'patch-auditConfigs-policy',
        'action': 'gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.setIamPolicy',
        'properties': {
          'resource': project_id,
          'policy': {'auditConfigs': [{
                      'auditLogConfigs': [{'logType':'DATA_READ'},{'logType':'ADMIN_READ'}],
                      'service': 'cloudkms.googleapis.com'}]},
          'updateMask': "auditConfigs,etag"
        }
    }])
shuainie-google commented 6 years ago

Hi @davidebelloni, one possible reason is the "etag" field in the 'updateMask' is not provided in the 'policy' section in 'patch-auditConfigs-policy'. Can you try removing 'etag' from the mask or use another action to get the etag and set it in 'patch-auditConfigs-policy' action?

davidebelloni commented 6 years ago

Hi @shuainie-google , removing etag doesn't resolve the issue. I'll try in the next days to use another action to get the etag and set it in 'patch-auditConfigs-policy' action and I'll tell you.

Thanks

shuainie-google commented 6 years ago

Hi @davidebelloni, did you got a chance to try getting etag using another action? Thanks

davidebelloni commented 6 years ago

Hi @shuainie-google , retrieving etag and appling iam policy now, after the first update, return:

ERROR: (gcloud.deployment-manager.deployments.update) Error in Operation [operation-1513673413960-560ad8e252143-a3103ea1-e1e31109]: errors:
- code: RESOURCE_ERROR
  location: /deployments/dtdd-appb2c-customers-test/resources/patch-auditConfigs-policy
  message: '{"ResourceType":"gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.setIamPolicy","ResourceErrorCode":"409","ResourceErrorMessage":{"code":409,"message":"There
    were concurrent policy changes. Please retry the whole read-modify-write with
    exponential backoff.","status":"ABORTED","statusMessage":"Conflict","requestPath":"https://cloudresourcemanager.googleapis.com/v1/projects/dtdd-appb2c-customers-test:setIamPolicy","httpMethod":"POST"}}'

The code was:

    resources.extend([{
        'name': 'get-etag-auditConfigs',
        'action': 'gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.getIamPolicy',
        'properties': {
            'resource': project_id,
        }, 'metadata': {
            'dependsOn': ['project']
        }
    }, {
        'name': 'patch-auditConfigs-policy',
        'action': 'gcp-types/cloudresourcemanager-v1:cloudresourcemanager.projects.setIamPolicy',
        'properties': {
          'resource': project_id,
          'policy': { 'etag' : '$(ref.get-etag-auditConfigs.etag)',
             'auditConfigs': [
            {'auditLogConfigs': [{'logType':'DATA_READ'},{'logType':'ADMIN_READ'}], 'service': 'cloudkms.googleapis.com'},
            {'auditLogConfigs': [{'logType':'DATA_READ'},], 'service': 'cloudsql.googleapis.com'}
            ]},
          'updateMask': 'auditConfigs,etag'
        },
        'metadata': {
            'dependsOn': ['get-etag-auditConfigs']
        }
    }])

I've typically two big problem with DM, the "read-modify-write" and the "CYCLIC_REFERENCES" errors. Now the second one is the error returned by DM with the previous code version O_o (that was working before the test). I needed to delete (with policy abandon) and recreate the deployment

It's quite difficult manage these errors as a DM customers.

Thanks

shuainie-google commented 6 years ago

Hi @davidebelloni , we don't support retry for certain set of actions within a deployment creation. One better way to do this is to add 'runtimePolicy': ['UPDATE_ALWAYS'] in the metadata of the "get-etag-auditConfigs" and "patch-auditConfigs-policy" actions like the sample here. Then retry by update the deployment with same config if you see "concurrent policy changes" error again. "UPDATE_ALWAYS" policy will ensure getting a new etag everytime. You can also remove the workaround here by removing the timestamp for the action name. This should fix the CYCLIC_REFERENCES error.

davidebelloni commented 6 years ago

Hi @shuainie-google , I've solved the issue with the code " 'runtimePolicy': ['UPDATE_ALWAYS']" proposed by you!

I hope to see this features more documented in the next future. Thanks for the help

shuainie-google commented 6 years ago

Cool. Thanks @davidebelloni for the update.