chore(test) - Update to use github actions (#210)

* Switch to using github actions for PR testing

Author: kddejong

.github/workflows/pr-ci.yml

@@ -0,0 +1,42 @@
+# This workflow will install Python dependencies, run tests and lint with a single version of Python
+# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions
+
+name: CloudFormation GO Plugin CI
+
+on:
+  push:
+    branches: [ master ]
+  pull_request:
+    branches: [ master ]
+
+jobs:
+  build:
+    env:
+      AWS_DEFAULT_REGION: us-east-1
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python: [3.6, 3.7, 3.8]
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-go@v3
+      with:
+        go-version: '>=1.17.0'
+    - name: Set up Python ${{ matrix.python }}
+      uses: actions/setup-python@v2
+      with:
+        python-version: ${{ matrix.python }}
+    - name: Install dependencies
+      run: |
+        pip install pre-commit
+        pip install --upgrade mypy 'attrs==19.2.0' -r https://raw.githubusercontent.com/aws-cloudformation/aws-cloudformation-rpdk/master/requirements.txt
+        go install github.com/go-critic/go-critic/cmd/gocritic@latest
+        go install golang.org/x/tools/cmd/goimports@latest
+        go install github.com/fzipp/gocyclo/cmd/gocyclo@latest
+        curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.50.1
+    - name: Install plugin
+      run: |
+        pip install .
+    - name: pre-commit checks
+      run: |
+        pre-commit run --all-files

.pre-commit-config.yaml

@@ -1,6 +1,6 @@
 repos:
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v2.0.0
+  rev: v4.3.0
   hooks:
   - id: check-case-conflict
   - id: end-of-file-fixer
@@ -17,14 +17,73 @@ repos:
   - id: check-merge-conflict
   - id: check-yaml
 - repo: https://github.com/pre-commit/mirrors-isort
-  rev: v4.3.21
+  rev: v5.10.1
   hooks:
   - id: isort
 - repo: https://github.com/ambv/black
-  rev: stable
+  rev: 22.10.0
   hooks:
   - id: black
+- repo: https://github.com/pycqa/flake8
+  rev: '5.0.4'
+  hooks:
+  - id: flake8
+    additional_dependencies:
+    - flake8-bugbear>=19.3.0
+    - flake8-builtins>=1.4.1
+    - flake8-commas>=2.0.0
+    - flake8-comprehensions>=2.1.0
+    - flake8-debugger>=3.1.0
+    - flake8-pep3101>=1.2.1
+    # language_version: python3.6
+    exclude: templates/
+- repo: https://github.com/pre-commit/pygrep-hooks
+  rev: v1.9.0
+  hooks:
+  - id: python-check-blanket-noqa
+  - id: python-check-mock-methods
+  - id: python-no-log-warn
+- repo: https://github.com/PyCQA/bandit
+  rev: "1.7.4"
+  hooks:
+  - id: bandit
+    files: ^python/
+    additional_dependencies:
+      - "importlib-metadata<5"  # https://github.com/PyCQA/bandit/issues/956
+- repo: local
+  hooks:
+  - id: pylint-local
+    name: pylint-local
+    description: Run pylint in the local virtualenv
+    entry: pylint "setup.py" "python/" "tests/"
+    language: system
+    # ignore all files, run on hard-coded modules instead
+    pass_filenames: false
+    always_run: true
+  - id: pytest-local
+    name: pytest-local
+    description: Run pytest in the local virtualenv
+    entry: >
+      pytest
+      --cov-report=term
+      --cov-report=html
+      --cov="rpdk.go"
+      --durations=5
+      "tests/"
+    language: system
+    # ignore all files, run on hard-coded modules instead
+    pass_filenames: false
+    always_run: true
 - repo: https://github.com/dnephin/pre-commit-golang
-  rev: master
+  rev: v0.5.1
   hooks:
     - id: go-fmt
+    # - id: go-vet
+    - id: go-imports
+    - id: go-cyclo
+      args: [-over=20]
+    - id: golangci-lint
+    - id: go-critic
+    - id: go-unit-tests
+    - id: go-build
+    - id: go-mod-tidy

.pylintrc

@@ -0,0 +1,25 @@
+[MASTER]
+
+ignore=CVS,models.py,handlers.py,hook_models.py,hook_handlers.py,target_model.py
+jobs=1
+persistent=yes
+
+[MESSAGES CONTROL]
+
+disable=
+    missing-docstring,          # not everything needs a docstring
+    fixme,                      # work in progress
+    bad-continuation,           # clashes with black
+    too-few-public-methods,     # triggers when inheriting
+    ungrouped-imports,          # clashes with isort
+    duplicate-code,             # broken, setup.py
+    wrong-import-order,         # isort has precedence
+
+[BASIC]
+
+good-names=e,ex,f,fp,i,j,k,v,n,_
+
+[FORMAT]
+
+indent-string='    '
+max-line-length=88

buildspec.yml

@@ -8,4 +8,4 @@ phases:
       - pip install pre-commit
   build:
     commands:
-      - pre-commit run --all-files
+      - pre-commit --version

cfn/cfn.go

@@ -107,10 +107,10 @@ func makeEventFunc(h Handler) eventFunc {
 			log.Printf("Error: %v", err)
 			m.PublishExceptionMetric(time.Now(), event.Action, err)
 		}
-		handlerFn, err := router(event.Action, h)
+		handlerFn, cfnErr := router(event.Action, h)
 		log.Printf("Handler received the %s action", event.Action)
-		if err != nil {
-			return re.report(event, "router error", err, serviceInternalError)
+		if cfnErr != nil {
+			return re.report(event, "router error", cfnErr, serviceInternalError)
 		}
 		if err := validateEvent(event); err != nil {
 			return re.report(event, "validation error", err, invalidRequestError)
@@ -158,7 +158,7 @@ func scrubFiles(dir string) error {
 
 // router decides which handler should be invoked based on the action
 // It will return a route or an error depending on the action passed in
-func router(a string, h Handler) (handlerFunc, error) {
+func router(a string, h Handler) (handlerFunc, cfnerr.Error) {
 	// Figure out which action was called and have a "catch-all"
 	switch a {
 	case createAction:
@@ -185,7 +185,7 @@ func invoke(handlerFn handlerFunc, request handler.Request, metricsPublisher *me
 
 	// Ask the goroutine to do some work for us.
 	go func() {
-		//start the timer
+		// start the timer
 		s := time.Now()
 		metricsPublisher.PublishInvocationMetric(time.Now(), string(action))
 

cfn/cfnerr/types.go

@@ -118,11 +118,15 @@ func (b baseError) OrigErrs() []error {
 
 // So that the Error interface type can be included as an anonymous field
 // in the requestError struct and not conflict with the error.Error() method.
+//
+//nolint:all
 type cfnError Error
 
 // A requestError wraps a request or service error.
 //
 // Composed of baseError for code, message, and original error.
+//
+//nolint:all
 type requestError struct {
 	cfnError
 	statusCode int
@@ -137,6 +141,8 @@ type requestError struct {
 // that may be meaningful.
 //
 // Also wraps original errors via the baseError.
+//
+//nolint:all
 func newRequestError(err Error, statusCode int, requestID string) *requestError {
 	return &requestError{
 		cfnError:   err,
@@ -147,6 +153,8 @@ func newRequestError(err Error, statusCode int, requestID string) *requestError
 
 // Error returns the string representation of the error.
 // Satisfies the error interface.
+//
+//nolint:all
 func (r requestError) Error() string {
 	extra := fmt.Sprintf("status code: %d, request id: %s",
 		r.statusCode, r.requestID)
@@ -155,22 +163,30 @@ func (r requestError) Error() string {
 
 // String returns the string representation of the error.
 // Alias for Error to satisfy the stringer interface.
+//
+//nolint:all
 func (r requestError) String() string {
 	return r.Error()
 }
 
 // StatusCode returns the wrapped status code for the error
+//
+//nolint:all
 func (r requestError) StatusCode() int {
 	return r.statusCode
 }
 
 // RequestID returns the wrapped requestID
+//
+//nolint:all
 func (r requestError) RequestID() string {
 	return r.requestID
 }
 
 // OrigErrs returns the original errors if one was set. An empty slice is
 // returned if no error was set.
+//
+//nolint:all
 func (r requestError) OrigErrs() []error {
 	if b, ok := r.cfnError.(BatchedErrors); ok {
 		return b.OrigErrs()
@@ -189,7 +205,7 @@ func (e errorList) Error() string {
 	// How do we want to handle the array size being zero
 	if size := len(e); size > 0 {
 		for i := 0; i < size; i++ {
-			msg += fmt.Sprintf("%s", e[i].Error())
+			msg += e[i].Error()
 			// We check the next index to see if it is within the slice.
 			// If it is, then we append a newline. We do this, because unit tests
 			// could be broken with the additional '\n'

cfn/context.go

@@ -53,6 +53,8 @@ func GetContextSession(ctx context.Context) (*session.Session, error) {
 
 // marshalCallback allows for a handler.ProgressEvent to be parsed into something
 // the RPDK can use to reinvoke the resource provider with the same context.
+//
+//nolint:all
 func marshalCallback(pevt *handler.ProgressEvent) (map[string]interface{}, int64) {
 	return map[string]interface{}(pevt.CallbackContext), pevt.CallbackDelaySeconds
 }

cfn/entry_test.go

@@ -6,8 +6,6 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
-
-	"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/cfnerr"
 )
 
 func TestMarshalling(t *testing.T) {
@@ -69,8 +67,7 @@ func TestRouter(t *testing.T) {
 
 	t.Run("Failed Path", func(t *testing.T) {
 		fn, err := router(unknownAction, &EmptyHandler{})
-		cfnErr := err.(cfnerr.Error)
-		if cfnErr != nil && cfnErr.Code() != invalidRequestError {
+		if err != nil && err.Code() != invalidRequestError {
 			t.Errorf("Unspecified error returned: %v", err)
 		} else if err == nil {
 			t.Errorf("There should have been an error")

cfn/event.go

@@ -51,6 +51,8 @@ func validateEvent(event *event) error {
 
 // resourceHandlerRequest is internal to the RPDK. It contains a number of fields that are for
 // internal contract testing use only.
+//
+//nolint:all
 type resourceHandlerRequest struct {
 	ClientRequestToken        string          `json:"clientRequestToken"`
 	DesiredResourceState      json.RawMessage `json:"desiredResourceState"`

cfn/handler/event.go

@@ -15,8 +15,8 @@ type ProgressEvent struct {
 	HandlerErrorCode string `json:"errorCode,omitempty"`
 
 	// Message which can be shown to callers to indicate the
-	//nature of a progress transition or callback delay; for example a message
-	//indicating "propagating to edge."
+	// nature of a progress transition or callback delay; for example a message
+	// indicating "propagating to edge."
 	Message string `json:"message,omitempty"`
 
 	// CallbackContext is an arbitrary datum which the handler can return in an

cfn/handler/handler_test.go

@@ -3,7 +3,6 @@ package handler
 import (
 	"testing"
 
-	"github.com/aws-cloudformation/cloudformation-cli-go-plugin/cfn/cfnerr"
 	"github.com/aws/aws-sdk-go/aws"
 )
 
@@ -54,8 +53,7 @@ func TestNewRequest(t *testing.T) {
 				t.Fatalf("Didn't throw an error")
 			}
 
-			cfnErr := err.(cfnerr.Error)
-			if cfnErr.Code() != bodyEmptyError {
+			if err.Code() != bodyEmptyError {
 				t.Fatalf("Wrong error returned: %v", err)
 			}
 		})
@@ -70,8 +68,7 @@ func TestNewRequest(t *testing.T) {
 				t.Fatalf("Didn't throw an error")
 			}
 
-			cfnErr := err.(cfnerr.Error)
-			if cfnErr.Code() != marshalingError {
+			if err.Code() != marshalingError {
 				t.Fatalf("Wrong error returned: %v", err)
 			}
 		})
@@ -88,8 +85,7 @@ func TestNewRequest(t *testing.T) {
 				t.Fatalf("Didn't throw an error")
 			}
 
-			cfnErr := err.(cfnerr.Error)
-			if cfnErr.Code() != marshalingError {
+			if err.Code() != marshalingError {
 				t.Fatalf("Wrong error returned: %v", err)
 			}
 		})

cfn/handler/request.go

@@ -76,7 +76,7 @@ func NewRequest(id string, ctx map[string]interface{}, requestCTX RequestContext
 
 // UnmarshalPrevious populates the provided interface
 // with the previous properties of the resource
-func (r *Request) UnmarshalPrevious(v interface{}) error {
+func (r *Request) UnmarshalPrevious(v interface{}) cfnerr.Error {
 	if len(r.previousResourcePropertiesBody) == 0 {
 		return nil
 	}
@@ -90,7 +90,7 @@ func (r *Request) UnmarshalPrevious(v interface{}) error {
 
 // Unmarshal populates the provided interface
 // with the current properties of the resource
-func (r *Request) Unmarshal(v interface{}) error {
+func (r *Request) Unmarshal(v interface{}) cfnerr.Error {
 	if len(r.resourcePropertiesBody) == 0 {
 		return cfnerr.New(bodyEmptyError, "Body is empty", nil)
 	}

cfn/metrics/publisher.go

@@ -16,19 +16,19 @@ import (
 const (
 	// MetricNameSpaceRoot is the Metric name space root.
 	MetricNameSpaceRoot = "AWS/CloudFormation"
-	//MetricNameHanderException  is a metric type.
+	// MetricNameHanderException  is a metric type.
 	MetricNameHanderException = "HandlerException"
-	//MetricNameHanderDuration is a metric type.
+	// MetricNameHanderDuration is a metric type.
 	MetricNameHanderDuration = "HandlerInvocationDuration"
-	//MetricNameHanderInvocationCount is a metric type.
+	// MetricNameHanderInvocationCount is a metric type.
 	MetricNameHanderInvocationCount = "HandlerInvocationCount"
-	//DimensionKeyAcionType  is the Action key in the dimension.
+	// DimensionKeyAcionType  is the Action key in the dimension.
 	DimensionKeyAcionType = "Action"
-	//DimensionKeyExceptionType  is the ExceptionType in the dimension.
+	// DimensionKeyExceptionType  is the ExceptionType in the dimension.
 	DimensionKeyExceptionType = "ExceptionType"
-	//DimensionKeyResourceType  is the ResourceType in the dimension.
+	// DimensionKeyResourceType  is the ResourceType in the dimension.
 	DimensionKeyResourceType = "ResourceType"
-	//ServiceInternalError ...
+	// ServiceInternalError ...
 	ServiceInternalError string = "ServiceInternal"
 )