Skip to content

Commit

Permalink
Issue argoproj#71: policy struct simplifications: remove bool pointer…
Browse files Browse the repository at this point in the history
… and template pointer
  • Loading branch information
jessesuen authored and wokeGit committed Nov 21, 2017
1 parent 5878643 commit c3d3bb9
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 32 deletions.
8 changes: 4 additions & 4 deletions saas/axops/src/applatix.io/axops/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func EnablePolicy() gin.HandlerFunc {

// Get user who enabled the policy
u := GetContextUser(c)
p.Enabled = utils.NewTrue()
p.Enabled = true
current_time := time.Now()
p.Status = fmt.Sprintf("Policy %s is enabled by %s on %s", p.Name, u.Username, current_time.Format("2006-01-02 15:04:05"))
p, axErr = p.Update()
Expand Down Expand Up @@ -198,7 +198,7 @@ func DisablePolicy() gin.HandlerFunc {

// Get user who disables the policy
u := GetContextUser(c)
p.Enabled = utils.NewFalse()
p.Enabled = false
current_time := time.Now()
p.Status = fmt.Sprintf("Policy %s is disabled by %s on %s", p.Name, u.Username, current_time.Format("2006-01-02 15:04:05"))

Expand Down Expand Up @@ -260,9 +260,9 @@ func sendNotificationForPolicy(p *policy.Policy) {
detail["Policy Repo"] = p.Repo
detail["Policy Branch"] = p.Branch
detail["Message"] = p.Status
if *(p.Enabled) == true {
if p.Enabled == true {
notification_center.Producer.SendMessage(notification_center.CodeEnabledPolicy, "", []string{}, detail)
} else if *(p.Enabled) == false {
} else {
notification_center.Producer.SendMessage(notification_center.CodeDisabledPolicy, "", []string{}, detail)
}
}
4 changes: 2 additions & 2 deletions saas/axops/src/applatix.io/axops/policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ const (
)

type Policy struct {
*template.PolicyTemplate
Enabled *bool `json:"enabled,omitempty"`
template.PolicyTemplate
Enabled bool `json:"enabled,omitempty"`
Status string `json:"status,omitempty"`
}

Expand Down
10 changes: 5 additions & 5 deletions saas/axops/src/applatix.io/axops/policy/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ import (

func (s *S) TestPolicyInsertUpdate(c *check.C) {
randStr := test.RandStr()
p := &policy.Policy{PolicyTemplate: &template.PolicyTemplate{}}
p := &policy.Policy{}
p.ID = utils.GenerateUUIDv1()
p.Name = randStr
p.Description = randStr
p.Repo = randStr
p.Branch = randStr
p.Template = randStr
p.Enabled = test.NewTrue()
p.Enabled = true
p.Notifications = []template.Notification{
template.Notification{
Whom: []string{"a", "b"},
Expand Down Expand Up @@ -51,12 +51,12 @@ func (s *S) TestPolicyInsertUpdate(c *check.C) {
c.Assert(copy.Repo, check.Equals, p.Repo)
c.Assert(copy.Branch, check.Equals, p.Branch)
c.Assert(copy.Template, check.Equals, p.Template)
c.Assert(*copy.Enabled, check.Equals, *p.Enabled)
c.Assert(copy.Enabled, check.Equals, p.Enabled)
c.Assert(len(copy.Notifications), check.Equals, len(p.Notifications))
c.Assert(len(copy.When), check.Equals, len(p.When))
c.Assert(len(copy.Arguments), check.Equals, len(p.Arguments))

p.Enabled = test.NewFalse()
p.Enabled = false
p.Description = "changed"
p, err = p.Update()
c.Assert(err, check.IsNil)
Expand All @@ -72,7 +72,7 @@ func (s *S) TestPolicyInsertUpdate(c *check.C) {
c.Assert(copy.Repo, check.Equals, p.Repo)
c.Assert(copy.Branch, check.Equals, p.Branch)
c.Assert(copy.Template, check.Equals, p.Template)
c.Assert(*copy.Enabled, check.Equals, *p.Enabled)
c.Assert(copy.Enabled, check.Equals, p.Enabled)
c.Assert(len(copy.Notifications), check.Equals, len(p.Notifications))
c.Assert(len(copy.When), check.Equals, len(p.When))
c.Assert(len(copy.Arguments), check.Equals, len(p.Arguments))
Expand Down
5 changes: 2 additions & 3 deletions saas/axops/src/applatix.io/axops/policy/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"applatix.io/axdb"
"applatix.io/axerror"
"applatix.io/axops/utils"
"applatix.io/template"
)

const (
Expand Down Expand Up @@ -55,15 +54,15 @@ type policyDB struct {
Branch string `json:"branch"`
Revision string `json:"revision"`
Template string `json:"template"`
Enabled *bool `json:"enabled"`
Enabled bool `json:"enabled"`
Body string `json:"body"`
Labels map[string]string `json:"labels"`
RepoBranch string `json:"repo_branch"`
Status string `json:"status"`
}

func (p *policyDB) policy() (*Policy, *axerror.AXError) {
policy := &Policy{PolicyTemplate: &template.PolicyTemplate{}}
policy := &Policy{}
if p.Body != "" {
err := json.Unmarshal([]byte(p.Body), policy)
if err != nil {
Expand Down
16 changes: 8 additions & 8 deletions saas/axops/src/applatix.io/axops/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ import (
func (s *S) TestPolicyGetList(c *check.C) {

randStr := "rand" + test.RandStr()
p1 := &policy.Policy{PolicyTemplate: &template.PolicyTemplate{}}
p1 := &policy.Policy{}
p1.Name = randStr
p1.Description = randStr
p1.Repo = randStr
p1.Branch = randStr
p1.Template = randStr
p1.Enabled = test.NewTrue()
p1.Enabled = true
p1.Notifications = []template.Notification{
template.Notification{
When: []string{"on_change"},
Expand All @@ -42,13 +42,13 @@ func (s *S) TestPolicyGetList(c *check.C) {
}

randStr = "rand" + test.RandStr()
p2 := &policy.Policy{PolicyTemplate: &template.PolicyTemplate{}}
p2 := &policy.Policy{}
p2.Name = randStr
p2.Description = randStr
p2.Repo = randStr
p2.Branch = randStr
p2.Template = randStr
p2.Enabled = test.NewFalse()
p2.Enabled = false
p2.Notifications = []template.Notification{}
p2.When = []template.When{
template.When{
Expand Down Expand Up @@ -170,13 +170,13 @@ func (s *S) TestPolicyGetList(c *check.C) {
func (s *S) TestPolicyEnableDisable(c *check.C) {

randStr := "rand" + test.RandStr()
p1 := &policy.Policy{PolicyTemplate: &template.PolicyTemplate{}}
p1 := &policy.Policy{}
p1.Name = randStr
p1.Description = randStr
p1.Repo = randStr
p1.Branch = randStr
p1.Template = randStr
p1.Enabled = test.NewFalse()
p1.Enabled = false
p1.Notifications = []template.Notification{
template.Notification{
When: []string{"on_change"},
Expand Down Expand Up @@ -210,7 +210,7 @@ func (s *S) TestPolicyEnableDisable(c *check.C) {
c.Assert(err, check.IsNil)
c.Assert(p, check.NotNil)
c.Assert(p.ID, check.Equals, p1.ID)
c.Assert(*p.Enabled, check.Equals, true)
c.Assert(p.Enabled, check.Equals, true)

_, err = axopsClient.Put("policies/"+p.ID+"/disable", nil)
c.Assert(err, check.IsNil)
Expand All @@ -219,5 +219,5 @@ func (s *S) TestPolicyEnableDisable(c *check.C) {
c.Assert(err, check.IsNil)
c.Assert(p, check.NotNil)
c.Assert(p.ID, check.Equals, p1.ID)
c.Assert(*p.Enabled, check.Equals, false)
c.Assert(p.Enabled, check.Equals, false)
}
4 changes: 2 additions & 2 deletions saas/axops/src/applatix.io/axops/yaml/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,11 @@ func GarbageCollectTemplatePolicyProjectFixture() {
for _, p := range policies {
if branchesMap[p.Repo+"_"+p.Branch] == nil {
utils.DebugLog.Printf("GC POLICIES: %v %v is gone, deleting the policy %v(%v).\n", p.Repo, p.Branch, p.Name, p.ID)
if p.Enabled != nil && *(p.Enabled) == true {
if p.Enabled {
// If a policy is enabled, but from the source code, it gets deleted or syntax becomes
// invalid for whatever reason, we cannot just delete it without notifying user. The
// following change will make the policy invalid and let the user figure out what to do.
p.Enabled = utils.NewFalse()
p.Enabled = false
p.Status = policy.InvalidStatus
_, e := p.Update()
if e != nil {
Expand Down
14 changes: 6 additions & 8 deletions saas/axops/src/applatix.io/axops/yaml/ingest.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ func updatePolicies(ctx *template.TemplateBuildContext) *axerror.AXError {

newPolicy := func(tmpl *template.PolicyTemplate) *policy.Policy {
return &policy.Policy{
PolicyTemplate: tmpl,
Enabled: utils.NewFalse(),
PolicyTemplate: *tmpl,
Enabled: false,
}
}

Expand All @@ -183,17 +183,15 @@ func updatePolicies(ctx *template.TemplateBuildContext) *axerror.AXError {
}
updated[new.GetName()] = true
updatedPolicy := newPolicy(new.(*template.PolicyTemplate))
if old.Enabled != nil {
updatedPolicy.Enabled = old.Enabled
}
updatedPolicy.Enabled = old.Enabled

// If the old policy is an invalid one, that means it was an enabled policy but for some
// reason gets deleted or becomes syntax invalid. The new change will put things back
// into order, which means makes the policy valid and enabled.
if old.Status == policy.InvalidStatus {
utils.DebugLog.Printf("Invalid policy %v (%v) becomes valid. Turn it back on\n", old, old.Enabled)
updatedPolicy.Status = ""
updatedPolicy.Enabled = utils.NewTrue()
updatedPolicy.Enabled = true

// Send notification to notification center
detail := map[string]interface{}{}
Expand Down Expand Up @@ -231,11 +229,11 @@ func updatePolicies(ctx *template.TemplateBuildContext) *axerror.AXError {
// When deleting policies through GC, we would like to handle the enabled policy gracefully just like updateYAML
for _, p := range toBeDeleted {
utils.DebugLog.Printf("Delete policy %v %v\n", p, p.Enabled)
if p.Enabled != nil && *(p.Enabled) == true {
if p.Enabled {
// If a policy is enabled, but from the source code, it gets deleted or syntax becomes
// invalid for whatever reason, we cannot just delete it without notifying user. The
// following change will make the policy invalid and let the user figure out what to do.
p.Enabled = utils.NewFalse()
p.Enabled = false
p.Status = policy.InvalidStatus
_, e := p.Update()
if e != nil {
Expand Down

0 comments on commit c3d3bb9

Please sign in to comment.