Skip to content

Commit

Permalink
Block dag deploy update for Cicd Enforced deployments (#1327)
Browse files Browse the repository at this point in the history
* Block dag deploy update for Cicd Enforced deployments

* Block deploy for ci cd enforcemenrt deployments

* Add tests for deploy

* Add deployment file tests

* Add deployment tests

* Fixing tests

* Adding more tests

* Fixing Lint errors

* Changing from error to a warning for enabling dag deploy

* updating tests to bump coverage
  • Loading branch information
kushalmalani authored Aug 1, 2023
1 parent 8e385a1 commit 4813005
Show file tree
Hide file tree
Showing 6 changed files with 303 additions and 5 deletions.
14 changes: 12 additions & 2 deletions cloud/deploy/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,13 @@ var (
airflowImageHandler = airflow.ImageHandlerInit
containerHandlerInit = airflow.ContainerHandlerInit
azureUploader = azure.Upload
canCiCdDeploy = deployment.CanCiCdDeploy
)

var (
errDagsParseFailed = errors.New("your local DAGs did not parse. Fix the listed errors or use `astro deploy [deployment-id] -f` to force deploy") //nolint:revive
envFileMissing = errors.New("Env file path is incorrect: ") //nolint:revive
errDagsParseFailed = errors.New("your local DAGs did not parse. Fix the listed errors or use `astro deploy [deployment-id] -f` to force deploy") //nolint:revive
envFileMissing = errors.New("Env file path is incorrect: ") //nolint:revive
errCiCdEnforcementUpdate = errors.New("cannot update dag deploy since ci/cd enforcement is enabled for this deployment. Please use API Tokens or API Keys instead")
)

var (
Expand All @@ -87,6 +89,7 @@ type deploymentInfo struct {
webserverURL string
dagDeployEnabled bool
deploymentType string
cicdEnforcement bool
}

type InputDeploy struct {
Expand Down Expand Up @@ -242,6 +245,12 @@ func Deploy(deployInput InputDeploy, client astro.Client, coreClient astrocore.C
return err
}

if deployInfo.cicdEnforcement {
if !canCiCdDeploy(c.Token) {
return errCiCdEnforcementUpdate
}
}

if deployInput.WsID != deployInfo.workspaceID {
fmt.Printf(invalidWorkspaceID, deployInput.WsID)
return nil
Expand Down Expand Up @@ -423,6 +432,7 @@ func getDeploymentInfo(deploymentID, wsID, deploymentName string, prompt bool, c
currentDeployment.DeploymentSpec.Webserver.URL,
currentDeployment.DagDeployEnabled,
currentDeployment.Type,
currentDeployment.APIKeyOnlyDeployments,
}, nil
}
deployInfo, err := getImageName(cloudDomain, deploymentID, client)
Expand Down
37 changes: 37 additions & 0 deletions cloud/deploy/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,43 @@ func TestDeployWithoutDagsDeploySuccess(t *testing.T) {
mockContainerHandler.AssertExpectations(t)
}

func TestDeployOnCiCdEnforcedDeployment(t *testing.T) {
os.Mkdir("./testfiles/dags", os.ModePerm)
path := "./testfiles/dags/test.py"
fileutil.WriteStringToFile(path, "testing")
mockCoreClient := new(astrocore_mocks.ClientWithResponsesInterface)

deployInput := InputDeploy{
Path: "./testfiles/",
RuntimeID: "",
WsID: ws,
Pytest: "parse",
EnvFile: "./testfiles/.env",
ImageName: "",
DeploymentName: "",
Prompt: true,
WaitForStatus: false,
Dags: false,
}
testUtil.InitTestConfig(testUtil.CloudPlatform)
config.CFG.ShowWarnings.SetHomeString("false")
mockClient := new(astro_mocks.Client)

canCiCdDeploy = func(astroAPIToken string) bool {
return false
}

mockClient.On("ListDeployments", org, ws).Return([]astro.Deployment{{ID: "test-id", Workspace: astro.Workspace{ID: ws}, DagDeployEnabled: true, APIKeyOnlyDeployments: true}}, nil).Once()

err := Deploy(deployInput, mockClient, mockCoreClient)
assert.ErrorIs(t, err, errCiCdEnforcementUpdate)

defer os.RemoveAll("./testfiles/dags/")

mockClient.AssertExpectations(t)
mockCoreClient.AssertExpectations(t)
}

func TestDeployWithDagsDeploySuccess(t *testing.T) {
os.Mkdir("./testfiles/dags", os.ModePerm)
path := "./testfiles/dags/test.py"
Expand Down
38 changes: 38 additions & 0 deletions cloud/deployment/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"os"
"sort"
"strconv"
"strings"
"time"

airflowversions "github.com/astronomer/astro-cli/airflow_versions"
Expand Down Expand Up @@ -35,6 +36,8 @@ var (
ErrNoDeploymentExists = errors.New("no deployment was found in this workspace")
// Monkey patched to write unit tests
createDeployment = Create
canCiCdDeploy = CanCiCdDeploy
parseToken = util.ParseAPIToken
CleanOutput = false
)

Expand Down Expand Up @@ -69,6 +72,23 @@ func newTableOutAll() *printutil.Table {
}
}

func CanCiCdDeploy(bearerToken string) bool {
token := strings.Split(bearerToken, " ")[1] // Stripping Bearer
// Parse the token to peek at the custom claims
claims, err := parseToken(token)
if err != nil {
fmt.Println("Unable to Parse Token")
return false
}

// Only API Tokens and API Keys have permissions
if len(claims.Permissions) > 0 {
return true
}

return false
}

// List all airflow deployments
func List(ws string, all bool, client astro.Client, out io.Writer) error {
c, err := config.GetCurrentContext()
Expand Down Expand Up @@ -649,6 +669,24 @@ func Update(deploymentID, label, ws, description, deploymentName, dagDeploy, exe
}
}

c, err := config.GetCurrentContext()
if err != nil {
return err
}

if deploymentUpdate.APIKeyOnlyDeployments && dagDeploy != "" {
if !canCiCdDeploy(c.Token) {
fmt.Printf("\nWarning: You are trying to update dag deploy setting on a deployment with ci-cd enforcement enabled. You will not be able to deploy your dags using the CLI and that dags will not be visible in the UI and new tasks will not start." +
"\nEither disable ci-cd enforcement or please cancel this operation and use API Tokens or API Keys instead.")
y, _ := input.Confirm("\n\nAre you sure you want to continue?")

if !y {
fmt.Println("Canceling Deployment update")
return nil
}
}
}

if dagDeploy == "enable" {
if currentDeployment.DagDeployEnabled {
fmt.Println("\nDAG deploys are already enabled for this Deployment. Your DAGs will continue to run as scheduled.")
Expand Down
79 changes: 79 additions & 0 deletions cloud/deployment/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
astro_mocks "github.com/astronomer/astro-cli/astro-client/mocks"
"github.com/astronomer/astro-cli/context"
testUtil "github.com/astronomer/astro-cli/pkg/testing"
"github.com/astronomer/astro-cli/pkg/util"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
)
Expand Down Expand Up @@ -1426,6 +1427,42 @@ func TestSelectCluster(t *testing.T) {
})
}

func TestCanCiCdDeploy(t *testing.T) {
permissions := []string{}
mockClaims := util.CustomClaims{
Permissions: permissions,
}

parseToken = func(astroAPIToken string) (*util.CustomClaims, error) {
return &mockClaims, nil
}

canDeploy := CanCiCdDeploy("bearer token")
assert.Equal(t, canDeploy, false)

parseToken = func(astroAPIToken string) (*util.CustomClaims, error) {
return nil, errMock
}
canDeploy = CanCiCdDeploy("bearer token")
assert.Equal(t, canDeploy, false)

permissions = []string{
"workspaceId:workspace-id",
"organizationId:org-ID",
"orgShortName:org-short-name",
}
mockClaims = util.CustomClaims{
Permissions: permissions,
}

parseToken = func(astroAPIToken string) (*util.CustomClaims, error) {
return &mockClaims, nil
}

canDeploy = CanCiCdDeploy("bearer token")
assert.Equal(t, canDeploy, true)
}

func TestUpdate(t *testing.T) { //nolint
testUtil.InitTestConfig(testUtil.CloudPlatform)
mockClient := new(astro_mocks.Client)
Expand Down Expand Up @@ -2053,6 +2090,48 @@ func TestUpdate(t *testing.T) { //nolint
mockClient.AssertExpectations(t)
})

t.Run("throw warning to enable dag deploy if ci-cd enforcement is enabled", func(t *testing.T) {
mockClient.On("GetDeploymentConfig").Return(astro.DeploymentConfig{
Components: astro.Components{
Scheduler: astro.SchedulerConfig{
AU: astro.AuConfig{
Default: 5,
Limit: 24,
},
Replicas: astro.ReplicasConfig{
Default: 1,
Minimum: 1,
Limit: 4,
},
},
},
RuntimeReleases: []astro.RuntimeRelease{
{
Version: "4.2.5",
},
},
}, nil).Once()
deploymentResp = astro.Deployment{
ID: "test-id",
DagDeployEnabled: true,
DeploymentSpec: astro.DeploymentSpec{
Executor: CeleryExecutor,
},
APIKeyOnlyDeployments: true,
}

canCiCdDeploy = func(astroAPIToken string) bool {
return false
}

mockClient.On("ListDeployments", org, ws).Return([]astro.Deployment{deploymentResp}, nil).Once()
defer testUtil.MockUserInput(t, "n")()
err := Update("test-id", "", ws, "", "", "enable", CeleryExecutor, "", "", 5, 3, []astro.WorkerQueue{}, true, nil, mockClient)
assert.NoError(t, err)

mockClient.AssertExpectations(t)
})

t.Run("update deployment to disable dag deploy", func(t *testing.T) {
mockClient.On("GetDeploymentConfig").Return(astro.DeploymentConfig{
Components: astro.Components{
Expand Down
22 changes: 19 additions & 3 deletions cloud/deployment/fromfile/fromfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/astronomer/astro-cli/cloud/organization"
"github.com/astronomer/astro-cli/cloud/workspace"
"github.com/astronomer/astro-cli/config"
"github.com/astronomer/astro-cli/pkg/input"
"github.com/ghodss/yaml"
)

Expand All @@ -31,6 +32,7 @@ var (
errNotFound = errors.New("does not exist")
errInvalidValue = errors.New("is not valid")
errNotPermitted = errors.New("is not permitted")
canCiCdDeploy = deployment.CanCiCdDeploy
)

const (
Expand Down Expand Up @@ -151,6 +153,20 @@ func CreateOrUpdate(inputFile, action string, client astro.Client, coreClient as
if err != nil {
return err
}

if formattedDeployment.Deployment.Configuration.APIKeyOnlyDeployments && updateInput.DagDeployEnabled {
if !canCiCdDeploy(c.Token) {
fmt.Printf("\nWarning: You are trying to update dag deploy setting on a deployment with ci-cd enforcement enabled. You will not be able to deploy your dags using the CLI and that dags will not be visible in the UI and new tasks will not start." +
"\nEither disable ci-cd enforcement or please cancel this operation and use API Tokens or API Keys instead.")
y, _ := input.Confirm("\n\nAre you sure you want to continue?")

if !y {
fmt.Println("Canceling Deployment update")
return nil
}
}
}

// update the deployment
createdOrUpdatedDeployment, err = client.UpdateDeployment(&updateInput)
if err != nil {
Expand Down Expand Up @@ -555,18 +571,18 @@ func hasAlertEmails(deploymentFromFile *inspect.FormattedDeployment) bool {
// It returns an error if it fails to update the alert emails for a deployment.
func createAlertEmails(deploymentFromFile *inspect.FormattedDeployment, deploymentID string, client astro.Client) (astro.DeploymentAlerts, error) {
var (
input astro.UpdateDeploymentAlertsInput
alertsInput astro.UpdateDeploymentAlertsInput
alertEmails []string
alerts astro.DeploymentAlerts
err error
)

alertEmails = deploymentFromFile.Deployment.AlertEmails
input = astro.UpdateDeploymentAlertsInput{
alertsInput = astro.UpdateDeploymentAlertsInput{
DeploymentID: deploymentID,
AlertEmails: alertEmails,
}
alerts, err = client.UpdateAlertEmails(input)
alerts, err = client.UpdateAlertEmails(alertsInput)
if err != nil {
return astro.DeploymentAlerts{}, err
}
Expand Down
Loading

0 comments on commit 4813005

Please sign in to comment.