added partial cloudflare api integration

engine/resources/cloudflare_dns.go

@@ -32,15 +32,12 @@ package resources
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/purpleidea/mgmt/engine"
 	"github.com/purpleidea/mgmt/engine/traits"
 	"github.com/purpleidea/mgmt/util/errwrap"
 
 	"github.com/cloudflare/cloudflare-go/v6"
-	"github.com/cloudflare/cloudflare-go/v6/dns"
-	"github.com/cloudflare/cloudflare-go/v6/option"
 	"github.com/cloudflare/cloudflare-go/v6/zones"
 )
 
@@ -48,74 +45,39 @@ func init() {
 	engine.RegisterResource("cloudflare:dns", func() engine.Res { return &CloudflareDNSRes{} })
 }
 
-// CloudflareDNSRes is a resource for managing DNS records in Cloudflare zones.
-// This resource uses the Cloudflare API to create, update, and delete DNS
-// records in a specified zone. It supports various record types including A,
-// AAAA, CNAME, MX, TXT, NS, and PTR records. The resource requires polling to
-// detect changes, as the Cloudflare API does not provide an event stream. The
-// Purge functionality allows enforcing that only managed DNS records exist in
-// the zone, removing any unmanaged records.
+// TODO: description of cloudflare_dns resource
 type CloudflareDNSRes struct {
 	traits.Base
-	traits.GraphQueryable
 	init *engine.Init
 
-	// APIToken is the Cloudflare API token used for authentication. This is
-	// required and must have the necessary permissions to manage DNS records
-	// in the specified zone.
 	APIToken string `lang:"apitoken"`
 
-	// Comment is an optional comment to attach to the DNS record. This is
-	// stored in Cloudflare and can be used for documentation purposes.
 	Comment string `lang:"comment"`
 
-	// Content is the value for the DNS record. This is required when State
-	// is "exists" unless Purge is true. The format depends on the record
-	// Type (e.g., IP address for A records, hostname for CNAME records).
 	Content string `lang:"content"`
 
-	// Priority is the priority value for records that support it (e.g., MX
-	// records). This is a pointer to distinguish between an unset value and
-	// a zero value.
-	Priority *float64 `lang:"priority"`
+	// using a *int64 here to help with disambiguating nil values
+	Priority *int64 `lang:"priority"`
 
-	// Proxied specifies whether the record should be proxied through
-	// Cloudflare's CDN. This is a pointer to distinguish between an unset
-	// value and false. Only applicable to certain record types.
+	// using a *bool here to help with disambiguating nil values
 	Proxied *bool `lang:"proxied"`
 
-	// Purge specifies whether to delete all DNS records in the zone that are
-	// not defined in the mgmt graph. When true, this resource will query the
-	// graph for other cloudflare:dns resources in the same zone and delete
-	// any records not managed by those resources.
-	Purge bool `lang:"purge"`
+	Purged bool `lang:"purged"`
 
-	// RecordName is the name of the DNS record (e.g., "www.example.com" or
-	// "@" for the zone apex). This is required.
 	RecordName string `lang:"record_name"`
 
-	// State determines whether the DNS record should exist or be absent.
-	// Valid values are "exists" (default) or "absent". When set to "absent",
-	// the record will be deleted if it exists.
 	State string `lang:"state"`
 
-	// TTL is the time-to-live value for the DNS record in seconds. Must be
-	// between 60 and 86400, or set to 1 for automatic TTL. Default is 1.
 	TTL int64 `lang:"ttl"`
 
-	// Type is the DNS record type (e.g., "A", "AAAA", "CNAME", "MX", "TXT",
-	// "NS", "SRV", "PTR"). This is required.
 	Type string `lang:"type"`
 
-	// Zone is the name of the Cloudflare zone (domain) where the DNS record
-	// should be managed (e.g., "example.com"). This is required.
 	Zone string `lang:"zone"`
 
 	client *cloudflare.Client
 	zoneID string
 }
 
-// Default returns some sensible defaults for this resource.
 func (obj *CloudflareDNSRes) Default() engine.Res {
 	return &CloudflareDNSRes{
 		State: "exists",
@@ -123,14 +85,13 @@ func (obj *CloudflareDNSRes) Default() engine.Res {
 	}
 }
 
-// Validate checks if the resource data structure was populated correctly.
 func (obj *CloudflareDNSRes) Validate() error {
 	if obj.RecordName == "" {
 		return fmt.Errorf("record name is required")
 	}
 
 	if obj.APIToken == "" {
-		return fmt.Errorf("api token is required")
+		return fmt.Errorf("API token is required")
 	}
 
 	if obj.Type == "" {
@@ -138,7 +99,7 @@ func (obj *CloudflareDNSRes) Validate() error {
 	}
 
 	if (obj.TTL < 60 || obj.TTL > 86400) && obj.TTL != 1 { // API requirement
-		return fmt.Errorf("ttl must be between 60s and 86400s, or set to 1")
+		return fmt.Errorf("TTL must be between 60s and 86400s, or set to 1")
 	}
 
 	if obj.Zone == "" {
@@ -153,21 +114,13 @@ func (obj *CloudflareDNSRes) Validate() error {
 		return fmt.Errorf("content is required when state is 'exists'")
 	}
 
-	if obj.Type == "MX" && obj.Priority == nil {
-		return fmt.Errorf("priority is required for MX records")
-	}
-
-	// cloudflare accepts ~4req/s so this is safe enough even when managing lots
-	// of records
-	if obj.MetaParams().Poll == 0 || obj.MetaParams().Poll < 60 {
-		return fmt.Errorf("cloudflare:dns requires polling, set Meta:poll param (e.g., 300s), min. 60s")
+	if obj.MetaParams().Poll == 0 {
+		return fmt.Errorf("cloudflare:dns requiers polling, set Meta:poll param	(e.g., 60 seconds)")
 	}
 
 	return nil
 }
 
-// Init runs some startup code for this resource. It initializes the Cloudflare
-// API client and validates that the specified zone exists.
 func (obj *CloudflareDNSRes) Init(init *engine.Init) error {
 	obj.init = init
 
@@ -175,26 +128,25 @@ func (obj *CloudflareDNSRes) Init(init *engine.Init) error {
 		option.WithAPIToken(obj.APIToken),
 	)
 
-	zoneListParams := zones.ZoneListParams{
-		Name: cloudflare.F(obj.Zone),
-	}
+	//TODO: does it make more sense to check it here or in CheckApply()?
+	//zoneListParams := zones.ZoneListParams{
+	//	name: cloudflare.F(obj.Zone),
+	//}
 
-	zoneList, err := obj.client.Zones.List(context.Background(), zoneListParams)
-	if err != nil {
-		return errwrap.Wrapf(err, "failed to list zones")
-	}
+	//zoneList, err := obj.client.Zones.List(context.Background(), zoneListParams)
+	//if err != nil {
+	//	return errwrap.Wrapf(err, "failed to list zones")
+	//}
 
-	if len(zoneList.Result) == 0 {
-		return fmt.Errorf("zone %s not found", obj.Zone)
-	}
+	//if len(zoneList.Result) == 0 {
+	//	return fmt.Errorf("zone %s not found", obj.Zone)
+	//}
 
-	obj.zoneID = zoneList.Result[0].ID
+	obj.zoneID = zoneList.Results[0].ID
 
 	return nil
 }
 
-// Cleanup is run by the engine to clean up after the resource is done. It
-// clears sensitive data and releases the API client connection.
 func (obj *CloudflareDNSRes) Cleanup() error {
 	obj.APIToken = ""
 	obj.client = nil
@@ -208,11 +160,22 @@ func (obj *CloudflareDNSRes) Watch(context.Context) error {
 	return fmt.Errorf("invalid Watch call: requires poll metaparam")
 }
 
-// CheckApply is the main convergence function for this resource. It checks the
-// current state of the DNS record against the desired state and applies changes
-// if necessary. If apply is false, it only checks if changes are needed. If
-// Purge is enabled, it will first delete any unmanaged records in the zone.
 func (obj *CloudflareDNSRes) CheckApply(ctx context.Context, apply bool) (bool, error) {
+	zone, err := obj.client.Zones.List(ctx, zones.ZoneListParams{
+		RecordName: cloudflare.F(obj.Zone),
+	})
+	if err != nil {
+		return false, fmt.Errorf(err)
+	}
+
+	if len(zone.Result) == 0 {
+		return false, fmt.Errorf("there's no zone registered with name %s", obj.Zone)
+	}
+
+	if len(zone.Result) > 1 {
+		return false, fmt.Errorf("there's more than one zone with name %s", obj.Zone)
+	}
+
 	// We start by checking the need for purging
 	if obj.Purge {
 		checkOK, err := obj.purgeCheckApply(ctx, apply)
@@ -224,14 +187,11 @@ func (obj *CloudflareDNSRes) CheckApply(ctx context.Context, apply bool) (bool,
 		}
 	}
 
-	// we're using `contains` so as to get the candidates, as `exact` might not
-	// give the expected results depending on how the user specified it.
+	// List existing records
 	listParams := dns.RecordListParams{
 		ZoneID: cloudflare.F(obj.zoneID),
-		Name: cloudflare.F(dns.RecordListParamsName{
-			Contains: cloudflare.F(obj.RecordName),
-		}),
-		Type: cloudflare.F(dns.RecordListParamsType(obj.Type)),
+		Name:   cloudflare.F(obj.RecordName),
+		Type:   cloudflare.F(dns.RecordListParamsType(obj.Type)),
 	}
 
 	recordList, err := obj.client.DNS.Records.List(ctx, listParams)
@@ -239,15 +199,10 @@ func (obj *CloudflareDNSRes) CheckApply(ctx context.Context, apply bool) (bool,
 		return false, errwrap.Wrapf(err, "failed to list DNS records")
 	}
 
-	// here we filter to find the exact match
-	recordExists := false
-	var record dns.RecordResponse
-	for _, r := range recordList.Result {
-		if obj.matchesRecordName(r.Name) {
-			record = r
-			recordExists = true
-			break
-		}
+	recordExists := len(records.Result) > 0
+	var record dns.Record
+	if recordExists {
+		record = recordList.Result[0]
 	}
 
 	switch obj.State {
@@ -285,7 +240,7 @@ func (obj *CloudflareDNSRes) CheckApply(ctx context.Context, apply bool) (bool,
 				ZoneID: cloudflare.F(obj.zoneID),
 			}
 
-			_, err := obj.client.DNS.Records.Delete(ctx, record.ID, deleteParams)
+			_, err := obj.client.DNS.Reords.Delete(ctx, record.ID, deleteParams)
 			if err != nil {
 				return false, errwrap.Wrapf(err, "failed to delete DNS record")
 			}
@@ -296,8 +251,6 @@ func (obj *CloudflareDNSRes) CheckApply(ctx context.Context, apply bool) (bool,
 	return true, nil
 }
 
-// Cmp compares two resources and returns an error if they differ. This is used
-// to determine if two resources are equivalent for graph operations.
 func (obj *CloudflareDNSRes) Cmp(r engine.Res) error {
 	if obj == nil && r == nil {
 		return nil
@@ -316,11 +269,8 @@ func (obj *CloudflareDNSRes) Cmp(r engine.Res) error {
 		return fmt.Errorf("apitoken differs")
 	}
 
-	if (obj.Proxied == nil) != (res.Proxied == nil) {
-		return fmt.Errorf("proxied values differ")
-	}
-
-	if obj.Proxied != nil && *obj.Proxied != *res.Proxied {
+	// check how this being a pointer influences this check
+	if obj.Proxied != res.Proxied {
 		return fmt.Errorf("proxied values differ")
 	}
 
@@ -328,7 +278,7 @@ func (obj *CloudflareDNSRes) Cmp(r engine.Res) error {
 		return fmt.Errorf("record name differs")
 	}
 
-	if obj.Purge != res.Purge {
+	if obj.Purged != res.Purged {
 		return fmt.Errorf("purge value differs")
 	}
 
@@ -356,22 +306,15 @@ func (obj *CloudflareDNSRes) Cmp(r engine.Res) error {
 		return fmt.Errorf("content param differs")
 	}
 
-	if (obj.Priority == nil) != (res.Priority == nil) {
-		return fmt.Errorf("the priority param differs")
-	}
-
-	if obj.Priority != nil && *obj.Priority != *res.Priority {
+	// check how this being a pointer influences this check
+	if obj.Priority != res.Priority {
 		return fmt.Errorf("the priority param differs")
 	}
 
 	return nil
 }
 
-// buildRecordParam creates the appropriate record parameter structure based on
-// the record type. This is a helper function used by buildNewRecordParam and
-// buildEditRecordParam.
-// TODO: double check the fields for each record, might have missed some
-func (obj *CloudflareDNSRes) buildRecordParam() (any, error) {
+func (obj *CloudflareDNSRes) buildRecordParam() dns.RecordNewParamsBodyUnion {
 	ttl := dns.TTL(obj.TTL)
 
 	switch obj.Type {
@@ -388,7 +331,7 @@ func (obj *CloudflareDNSRes) buildRecordParam() (any, error) {
 		if obj.Comment != "" {
 			param.Comment = cloudflare.F(obj.Comment)
 		}
-		return param, nil
+		return param
 
 	case "AAAA":
 		param := dns.AAAARecordParam{
@@ -403,7 +346,7 @@ func (obj *CloudflareDNSRes) buildRecordParam() (any, error) {
 		if obj.Comment != "" {
 			param.Comment = cloudflare.F(obj.Comment)
 		}
-		return param, nil
+		return param
 
 	case "CNAME":
 		param := dns.CNAMERecordParam{
@@ -418,7 +361,7 @@ func (obj *CloudflareDNSRes) buildRecordParam() (any, error) {
 		if obj.Comment != "" {
 			param.Comment = cloudflare.F(obj.Comment)
 		}
-		return param, nil
+		return param
 
 	case "MX":
 		param := dns.MXRecordParam{
@@ -436,7 +379,7 @@ func (obj *CloudflareDNSRes) buildRecordParam() (any, error) {
 		if obj.Comment != "" {
 			param.Comment = cloudflare.F(obj.Comment)
 		}
-		return param, nil
+		return param
 
 	case "TXT":
 		param := dns.TXTRecordParam{
@@ -451,7 +394,7 @@ func (obj *CloudflareDNSRes) buildRecordParam() (any, error) {
 		if obj.Comment != "" {
 			param.Comment = cloudflare.F(obj.Comment)
 		}
-		return param, nil
+		return param
 
 	case "NS":
 		param := dns.NSRecordParam{
@@ -466,7 +409,25 @@ func (obj *CloudflareDNSRes) buildRecordParam() (any, error) {
 		if obj.Comment != "" {
 			param.Comment = cloudflare.F(obj.Comment)
 		}
-		return param, nil
+		return param
+
+	case "SRV":
+		param := dns.SRVRecordParam{
+			Name:    cloudflare.F(obj.RecordName),
+			Type:    cloudflare.F(dns.SRVRecordTypeSRV),
+			Content: cloudflare.F(obj.Content),
+			TTL:     cloudflare.F(ttl),
+		}
+		if obj.Proxied != nil {
+			param.Proxied = cloudflare.F(*obj.Proxied)
+		}
+		if obj.Priority != nil {
+			param.Priority = cloudflare.F(*obj.Priority)
+		}
+		if obj.Comment != "" {
+			param.Comment = cloudflare.F(obj.Comment)
+		}
+		return param
 
 	case "PTR":
 		param := dns.PTRRecordParam{
@@ -481,48 +442,22 @@ func (obj *CloudflareDNSRes) buildRecordParam() (any, error) {
 		if obj.Comment != "" {
 			param.Comment = cloudflare.F(obj.Comment)
 		}
-		return param, nil
+		return param
 
-	default:
-		return nil, fmt.Errorf("record type %s is not supported", obj.Type)
+	default: // we should return something else here, need to investigate
 
 	}
 }
 
-// buildNewRecordParam creates the appropriate record parameter for creating new
-// records.
-func (obj *CloudflareDNSRes) buildNewRecordParam() (dns.RecordNewParamsBodyUnion, error) {
-	result, err := obj.buildRecordParam()
-	if err != nil {
-		return nil, err
-	}
-	return result.(dns.RecordNewParamsBodyUnion), nil
-}
-
-// buildEditRecordParam creates the appropriate record parameter for editing
-// records.
-func (obj *CloudflareDNSRes) buildEditRecordParam() (dns.RecordEditParamsBodyUnion, error) {
-	result, err := obj.buildRecordParam()
-	if err != nil {
-		return nil, err
-	}
-	return result.(dns.RecordEditParamsBodyUnion), nil
-}
-
-// createRecord creates a new DNS record in Cloudflare using the resource's
-// parameters.
 func (obj *CloudflareDNSRes) createRecord(ctx context.Context) error {
-	recordParams, err := obj.buildNewRecordParam()
-	if err != nil {
-		return err
-	}
+	recordParams := obj.buildRecordParam()
 
 	createParams := dns.RecordNewParams{
 		ZoneID: cloudflare.F(obj.zoneID),
 		Body:   recordParams,
 	}
 
-	_, err = obj.client.DNS.Records.New(ctx, createParams)
+	_, err := obj.client.DNS.Records.New(ctx, createParams)
 	if err != nil {
 		return errwrap.Wrapf(err, "failed to create dns record")
 	}
@@ -530,20 +465,15 @@ func (obj *CloudflareDNSRes) createRecord(ctx context.Context) error {
 	return nil
 }
 
-// updateRecord updates an existing DNS record in Cloudflare with the resource's
-// parameters.
 func (obj *CloudflareDNSRes) updateRecord(ctx context.Context, recordID string) error {
-	recordParams, err := obj.buildEditRecordParam()
-	if err != nil {
-		return err
-	}
+	recordParams := obj.buildRecordParam()
 
 	editParams := dns.RecordEditParams{
 		ZoneID: cloudflare.F(obj.zoneID),
 		Body:   recordParams,
 	}
 
-	_, err = obj.client.DNS.Records.Edit(ctx, recordID, editParams)
+	_, err := obj.client.DNS.Records.Edit(ctx, recordID, editParams)
 	if err != nil {
 		return errwrap.Wrapf(err, "failed to update dns record")
 	}
@@ -551,9 +481,7 @@ func (obj *CloudflareDNSRes) updateRecord(ctx context.Context, recordID string)
 	return nil
 }
 
-// needsUpdate compares the current DNS record with the desired state and
-// returns true if an update is needed.
-func (obj *CloudflareDNSRes) needsUpdate(record dns.RecordResponse) bool {
+func (obj *CloudflareDNSRes) needsUpdate(record dns.Record) bool {
 	if obj.Content != record.Content {
 		return true
 	}
@@ -562,143 +490,23 @@ func (obj *CloudflareDNSRes) needsUpdate(record dns.RecordResponse) bool {
 		return true
 	}
 
-	if obj.Proxied != nil {
-		if *obj.Proxied != record.Proxied {
+	if obj.Proxied != nil && record.Proxied != nil {
+		if *obj.Proxied != *record.Proxied {
 			return true
 		}
 	}
 
-	if obj.Priority != nil {
-		if *obj.Priority != record.Priority {
+	if obj.Priority != nil && record.Priority != nil {
+		if *obj.Priority != *record.Priority {
 			return true
 		}
 	}
 
-	if obj.Comment != "" && obj.Comment != record.Comment {
+	if obj.Comment != record.Comment {
 		return true
 	}
 
-	//TODO: add more checks?
+	// TODO add more checks?
 
 	return false
-
-}
-
-// purgeCheckApply deletes all DNS records in the zone that are not defined in
-// the mgmt graph. It queries the graph for other cloudflare:dns resources in
-// the same zone and builds an exclusion list. If apply is false, it only checks
-// if purge is needed.
-func (obj *CloudflareDNSRes) purgeCheckApply(ctx context.Context, apply bool) (bool, error) {
-	listParams := dns.RecordListParams{
-		ZoneID: cloudflare.F(obj.zoneID),
-	}
-
-	iter := obj.client.DNS.Records.ListAutoPaging(ctx, listParams)
-
-	allRecords := []dns.RecordResponse{}
-	for iter.Next() {
-		allRecords = append(allRecords, iter.Current())
-	}
-	if err := iter.Err(); err != nil {
-		return false, errwrap.Wrapf(err, "failed to list dns records for purge")
-	}
-
-	excludes := make(map[string]bool)
-
-	graph, err := obj.init.FilteredGraph()
-	if err != nil {
-		return false, errwrap.Wrapf(err, "can't read the filtered graph")
-	}
-
-	for _, vertex := range graph.Vertices() {
-		res, ok := vertex.(engine.Res)
-		if !ok {
-			return false, fmt.Errorf("not a resource")
-		}
-		if res.Kind() != "cloudflare:dns" {
-			continue // we only want cloudflare dns resources
-		}
-		if res.Name() == obj.Name() {
-			continue // skip self
-		}
-
-		cfRes, ok := res.(*CloudflareDNSRes)
-		if !ok {
-			return false, fmt.Errorf("wrong resource type")
-		}
-
-		if cfRes.Zone == obj.Zone {
-			recordKey := fmt.Sprintf("%s:%s:%s", cfRes.RecordName, cfRes.Type,
-				cfRes.Content)
-			if cfRes.Priority != nil {
-				// corner case for MX records which require priority set
-				recordKey = fmt.Sprintf("%s:%g", recordKey, *cfRes.Priority)
-			}
-			excludes[recordKey] = true
-		}
-	}
-
-	checkOK := true
-
-	for _, record := range allRecords {
-		recordKey := fmt.Sprintf("%s:%s:%s", record.Name, record.Type,
-			record.Content)
-		if record.Priority != 0 {
-			recordKey = fmt.Sprintf("%s:%g", recordKey, record.Priority)
-		}
-
-		if excludes[recordKey] {
-			continue
-		}
-
-		if apply {
-			deleteParams := dns.RecordDeleteParams{
-				ZoneID: cloudflare.F(obj.zoneID),
-			}
-			_, err := obj.client.DNS.Records.Delete(ctx, record.ID, deleteParams)
-			if err != nil {
-				return false, errwrap.Wrapf(err, "failed to purge %s", recordKey)
-			}
-		} else {
-			checkOK = false
-		}
-	}
-
-	return checkOK, nil
-}
-
-// GraphQueryAllowed returns nil if you're allowed to query the graph. This
-// function accepts information about the requesting resource so we can
-// determine the access with some form of fine-grained control.
-func (obj *CloudflareDNSRes) GraphQueryAllowed(opts ...engine.GraphQueryableOption) error {
-	options := &engine.GraphQueryableOptions{} // default options
-	options.Apply(opts...)                     // apply the options
-	if options.Kind != "cloudflare:dns" {
-		return fmt.Errorf("only other cloudflare dns resources can access this info")
-	}
-	return nil
-}
-
-// matchesRecordName checks if a record name from the API matches our desired
-// record name. Handles both FQDN (www.example.com) and short form (www)
-// comparisons.
-func (obj *CloudflareDNSRes) matchesRecordName(apiRecordName string) bool {
-	desired := obj.normalizeRecordName(obj.RecordName)
-	actual := obj.normalizeRecordName(apiRecordName)
-	return desired == actual
-}
-
-// normalizeRecordName converts a record name to a consistent format for
-// comparison. Converts to FQDN format (e.g., "www" -> "www.example.com", "@" ->
-// "example.com")
-func (obj *CloudflareDNSRes) normalizeRecordName(name string) string {
-	if name == "@" || name == obj.Zone {
-		return obj.Zone
-	}
-
-	if strings.HasSuffix(name, "."+obj.Zone) {
-		return name
-	}
-
-	return name + "." + obj.Zone
 }

engine/resources/exec.go

@@ -127,9 +127,9 @@ type ExecRes struct {
 	WatchShell string `lang:"watchshell" yaml:"watchshell"`
 
 	// IfCmd is the command that runs to guard against running the Cmd. If
-	// this command succeeds, then Cmd *will not* be blocked from running.
-	// If this command returns a non-zero result, then the Cmd will not be
-	// run. Any error scenario or timeout will cause the resource to error.
+	// this command succeeds, then Cmd *will* be run. If this command
+	// returns a non-zero result, then the Cmd will not be run. Any error
+	// scenario or timeout will cause the resource to error.
 	IfCmd string `lang:"ifcmd" yaml:"ifcmd"`
 
 	// IfCwd is the Cwd for the IfCmd. See the docs for Cwd.
@@ -145,19 +145,6 @@ type ExecRes struct {
 	// does!)
 	IfEquals *string `lang:"ifequals" yaml:"ifequals"`
 
-	// NIfCmd is the command that runs to guard against running the Cmd. If
-	// this command succeeds, then Cmd *will* be blocked from running. If
-	// this command returns a non-zero result, then the Cmd will be allowed
-	// to run if not blocked by anything else. This is the opposite of the
-	// IfCmd.
-	NIfCmd string `lang:"nifcmd" yaml:"nifcmd"`
-
-	// NIfCwd is the Cwd for the NIfCmd. See the docs for Cwd.
-	NIfCwd string `lang:"nifcwd" yaml:"nifcwd"`
-
-	// NIfShell is the Shell for the NIfCmd. See the docs for Shell.
-	NIfShell string `lang:"nifshell" yaml:"nifshell"`
-
 	// Creates is the absolute file path to check for before running the
 	// main cmd. If this path exists, then the cmd will not run. More
 	// precisely we attempt to `stat` the file, so it must succeed for a
@@ -482,7 +469,7 @@ func (obj *ExecRes) CheckApply(ctx context.Context, apply bool) (bool, error) {
 			cmdName = obj.IfShell // usually bash, or sh
 			cmdArgs = []string{"-c", obj.IfCmd}
 		}
-		cmd := exec.CommandContext(ctx, cmdName, cmdArgs...)
+		cmd := exec.Command(cmdName, cmdArgs...)
 		cmd.Dir = obj.IfCwd // run program in pwd if ""
 		// ignore signals sent to parent process (we're in our own group)
 		cmd.SysProcAttr = &syscall.SysProcAttr{
@@ -548,93 +535,6 @@ func (obj *ExecRes) CheckApply(ctx context.Context, apply bool) (bool, error) {
 		}
 	}
 
-	if obj.NIfCmd != "" { // opposite of the ifcmd check
-		var cmdName string
-		var cmdArgs []string
-		if obj.NIfShell == "" {
-			// call without a shell
-			// FIXME: are there still whitespace splitting issues?
-			split := strings.Fields(obj.NIfCmd)
-			cmdName = split[0]
-			//d, _ := os.Getwd() // TODO: how does this ever error ?
-			//cmdName = path.Join(d, cmdName)
-			cmdArgs = split[1:]
-		} else {
-			cmdName = obj.NIfShell // usually bash, or sh
-			cmdArgs = []string{"-c", obj.NIfCmd}
-		}
-		cmd := exec.CommandContext(ctx, cmdName, cmdArgs...)
-		cmd.Dir = obj.NIfCwd // run program in pwd if ""
-		// ignore signals sent to parent process (we're in our own group)
-		cmd.SysProcAttr = &syscall.SysProcAttr{
-			Setpgid: true,
-			Pgid:    0,
-		}
-
-		// if we have an user and group, use them
-		var err error
-		if cmd.SysProcAttr.Credential, err = obj.getCredential(); err != nil {
-			return false, errwrap.Wrapf(err, "error while setting credential")
-		}
-
-		var out splitWriter
-		out.Init()
-		cmd.Stdout = out.Stdout
-		cmd.Stderr = out.Stderr
-
-		err = cmd.Run()
-		if err == nil {
-			obj.init.Logf("nifcmd: %s", strings.Join(cmd.Args, " "))
-			obj.init.Logf("nifcmd exited with: %d, skipping cmd", 0)
-			s := out.String()
-			if s == "" {
-				obj.init.Logf("nifcmd out empty!")
-			} else {
-				obj.init.Logf("nifcmd out:")
-				obj.init.Logf("%s", s)
-			}
-
-			//if err := obj.checkApplyWriteCache(); err != nil {
-			//	return false, err
-			//}
-			obj.safety()
-			if err := obj.send(); err != nil {
-				return false, err
-			}
-			return true, nil // don't run
-		}
-
-		exitErr, ok := err.(*exec.ExitError) // embeds an os.ProcessState
-		if !ok {
-			// command failed in some bad way
-			return false, errwrap.Wrapf(err, "nifcmd failed in some bad way")
-		}
-		pStateSys := exitErr.Sys() // (*os.ProcessState) Sys
-		wStatus, ok := pStateSys.(syscall.WaitStatus)
-		if !ok {
-			return false, errwrap.Wrapf(err, "could not get exit status of nifcmd")
-		}
-		exitStatus := wStatus.ExitStatus()
-		if exitStatus == 0 {
-			// i'm not sure if this could happen
-			return false, errwrap.Wrapf(err, "unexpected nifcmd exit status of zero")
-		}
-
-		obj.init.Logf("nifcmd: %s", strings.Join(cmd.Args, " "))
-		obj.init.Logf("nifcmd exited with: %d, not skipping cmd", exitStatus)
-		if s := out.String(); s == "" {
-			obj.init.Logf("nifcmd out empty!")
-		} else {
-			obj.init.Logf("nifcmd out:")
-			obj.init.Logf("%s", s)
-		}
-
-		//if obj.NIfEquals != nil && *obj.NIfEquals == s {
-		//	obj.init.Logf("nifequals matched")
-		//	return true, nil // don't run
-		//}
-	}
-
 	if obj.Creates != "" { // gate the extra syscall
 		if _, err := os.Stat(obj.Creates); err == nil {
 			obj.init.Logf("creates file exists, skipping cmd")
@@ -817,7 +717,7 @@ func (obj *ExecRes) CheckApply(ctx context.Context, apply bool) (bool, error) {
 			cmdName = obj.DoneShell // usually bash, or sh
 			cmdArgs = []string{"-c", obj.DoneCmd}
 		}
-		cmd := exec.CommandContext(ctx, cmdName, cmdArgs...)
+		cmd := exec.Command(cmdName, cmdArgs...)
 		cmd.Dir = obj.DoneCwd // run program in pwd if ""
 		// ignore signals sent to parent process (we're in our own group)
 		cmd.SysProcAttr = &syscall.SysProcAttr{
@@ -1010,16 +910,6 @@ func (obj *ExecRes) Cmp(r engine.Res) error {
 		return errwrap.Wrapf(err, "the IfEquals differs")
 	}
 
-	if obj.NIfCmd != res.NIfCmd {
-		return fmt.Errorf("the NIfCmd differs")
-	}
-	if obj.NIfCwd != res.NIfCwd {
-		return fmt.Errorf("the NIfCwd differs")
-	}
-	if obj.NIfShell != res.NIfShell {
-		return fmt.Errorf("the NIfShell differs")
-	}
-
 	if obj.Creates != res.Creates {
 		return fmt.Errorf("the Creates differs")
 	}
@@ -1066,7 +956,6 @@ type ExecUID struct {
 	Cmd      string
 	WatchCmd string
 	IfCmd    string
-	NIfCmd   string
 	DoneCmd  string
 	// TODO: add more elements here
 }
@@ -1157,7 +1046,6 @@ func (obj *ExecRes) UIDs() []engine.ResUID {
 		Cmd:      obj.getCmd(),
 		WatchCmd: obj.WatchCmd,
 		IfCmd:    obj.IfCmd,
-		NIfCmd:   obj.NIfCmd,
 		DoneCmd:  obj.DoneCmd,
 		// TODO: add more params here
 	}
@@ -1252,11 +1140,6 @@ func (obj *ExecRes) cmdFiles() []string {
 	} else if sp := strings.Fields(obj.IfCmd); len(sp) > 0 {
 		paths = append(paths, sp[0])
 	}
-	if obj.NIfShell != "" {
-		paths = append(paths, obj.NIfShell)
-	} else if sp := strings.Fields(obj.NIfCmd); len(sp) > 0 {
-		paths = append(paths, sp[0])
-	}
 	if obj.DoneShell != "" {
 		paths = append(paths, obj.DoneShell)
 	} else if sp := strings.Fields(obj.DoneCmd); len(sp) > 0 {

engine/resources/group.go

@@ -197,7 +197,7 @@ func (obj *GroupRes) CheckApply(ctx context.Context, apply bool) (bool, error) {
 		cmdName = "groupdel"
 	}
 
-	cmd := exec.CommandContext(ctx, cmdName, args...)
+	cmd := exec.Command(cmdName, args...)
 	cmd.SysProcAttr = &syscall.SysProcAttr{
 		Setpgid: true,
 		Pgid:    0,

engine/resources/resources_test.go

@@ -661,7 +661,7 @@ func TestResources1(t *testing.T) {
 				}
 
 				t.Logf("test #%d: running CheckApply", index)
-				checkOK, err := res.CheckApply(context.TODO(), true) // no noop!
+				checkOK, err := res.CheckApply(doneCtx, true) // no noop!
 				if err != nil {
 					t.Errorf("test #%d: FAIL", index)
 					t.Errorf("test #%d: CheckApply failed: %s", index, err.Error())

engine/resources/sysctl.go

@@ -287,7 +287,7 @@ func (obj *SysctlRes) runtimeCheckApply(ctx context.Context, apply bool) (bool,
 	b, err := os.ReadFile(obj.toPath())
 	if err != nil && !os.IsNotExist(err) {
 		// system or permissions error?
-		return false, err
+		return false, nil
 	}
 	if err == nil && bytes.Equal(expected, b) {
 		return true, nil // we match!
@@ -323,7 +323,7 @@ func (obj *SysctlRes) persistCheckApply(ctx context.Context, apply bool) (bool,
 	b, err := os.ReadFile(obj.getFilename())
 	if err != nil && !os.IsNotExist(err) {
 		// system or permissions error?
-		return false, err
+		return false, nil
 	}
 	if err == nil && bytes.Equal(expected, b) {
 		return true, nil // we match!

engine/resources/virt_builder.go

@@ -36,7 +36,6 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
-	"os/user"
 	"path"
 	"path/filepath"
 	"runtime"
@@ -622,15 +621,6 @@ func (obj *VirtBuilderRes) CheckApply(ctx context.Context, apply bool) (bool, er
 
 	cmd := exec.CommandContext(ctx, cmdName, cmdArgs...)
 
-	usr, err := user.Current()
-	if err != nil {
-		return false, err
-	}
-	// FIXME: consider building this from an empty environment?
-	cmd.Env = append(os.Environ(),
-		fmt.Sprintf("HOME=%s", usr.HomeDir),
-	)
-
 	// ignore signals sent to parent process (we're in our own group)
 	cmd.SysProcAttr = &syscall.SysProcAttr{
 		Setpgid: true,
@@ -648,7 +638,7 @@ func (obj *VirtBuilderRes) CheckApply(ctx context.Context, apply bool) (bool, er
 		return false, errwrap.Wrapf(err, "error starting cmd")
 	}
 
-	cmderr := cmd.Wait() // we can unblock this with the timeout
+	err := cmd.Wait() // we can unblock this with the timeout
 	out := b.String()
 
 	p := path.Join(obj.varDir, fmt.Sprintf("%d.log", start))
@@ -658,7 +648,7 @@ func (obj *VirtBuilderRes) CheckApply(ctx context.Context, apply bool) (bool, er
 		}
 	}
 
-	if cmderr == nil {
+	if err == nil {
 		obj.init.Logf("built image successfully!")
 		return false, nil // success!
 	}
@@ -677,7 +667,7 @@ func (obj *VirtBuilderRes) CheckApply(ctx context.Context, apply bool) (bool, er
 		obj.init.Logf("deleted partial output")
 	}
 
-	exitErr, ok := cmderr.(*exec.ExitError) // embeds an os.ProcessState
+	exitErr, ok := err.(*exec.ExitError) // embeds an os.ProcessState
 	if !ok {
 		// command failed in some bad way
 		return false, errwrap.Wrapf(err, "cmd failed in some bad way")

examples/lang/exec-if.mcl

@@ -1,20 +0,0 @@
-exec "i will run 1" {
-	cmd => "/usr/bin/echo i WILL run",
-	ifcmd => "/usr/bin/true",
-}
-
-exec "i will not run 2" {
-	cmd => "/usr/bin/echo i will NOT run",
-	ifcmd => "/usr/bin/false",
-}
-
-# nifcmd exited with: 0, skipping cmd
-exec "i will not run 1" {
-	cmd => "/usr/bin/echo i will NOT run",
-	nifcmd => "/usr/bin/true",
-}
-
-exec "i will run 2" {
-	cmd => "/usr/bin/echo i WILL run",
-	nifcmd => "/usr/bin/false",
-}

go.mod

@@ -70,7 +70,6 @@ require (
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/chappjc/logrus-prefix v0.0.0-20180227015900-3a1d64819adb // indirect
 	github.com/cloudflare/circl v1.5.0 // indirect
-	github.com/cloudflare/cloudflare-go/v6 v6.1.0 // indirect
 	github.com/cloudwego/base64x v0.1.5 // indirect
 	github.com/containerd/log v0.1.0 // indirect
 	github.com/coreos/go-semver v0.3.1 // indirect
@@ -157,10 +156,6 @@ require (
 	github.com/spf13/cobra v1.8.1 // indirect
 	github.com/spf13/pflag v1.0.6-0.20250109003754-5ca813443bd2 // indirect
 	github.com/stmcginnis/gofish v0.20.0 // indirect
-	github.com/tidwall/gjson v1.14.4 // indirect
-	github.com/tidwall/match v1.1.1 // indirect
-	github.com/tidwall/pretty v1.2.1 // indirect
-	github.com/tidwall/sjson v1.2.5 // indirect
 	github.com/tmc/grpc-websocket-proxy v0.0.0-20220101234140-673ab2c3ae75 // indirect
 	github.com/twitchyliquid64/golang-asm v0.15.1 // indirect
 	github.com/u-root/uio v0.0.0-20240224005618-d2acac8f3701 // indirect

go.sum

@@ -68,8 +68,6 @@ github.com/circonus-labs/circonusllhist v0.1.3/go.mod h1:kMXHVDlOchFAehlya5ePtbp
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/cloudflare/circl v1.5.0 h1:hxIWksrX6XN5a1L2TI/h53AGPhNHoUBo+TD1ms9+pys=
 github.com/cloudflare/circl v1.5.0/go.mod h1:uddAzsPgqdMAYatqJ0lsjX1oECcQLIlRpzZh3pJrofs=
-github.com/cloudflare/cloudflare-go/v6 v6.1.0 h1:208leV/QEyIZuxFKNk3ztiOh4PeNW/qvLHvzafcbpjI=
-github.com/cloudflare/cloudflare-go/v6 v6.1.0/go.mod h1:Lj3MUqjvKctXRpdRhLQxZYRrNZHuRs0XYuH8JtQGyoI=
 github.com/cloudwego/base64x v0.1.5 h1:XPciSp1xaq2VCSt6lF0phncD4koWyULpl5bUxbfCyP4=
 github.com/cloudwego/base64x v0.1.5/go.mod h1:0zlkT4Wn5C6NdauXdJRhSKRlJvmclQ1hhJgA0rcu/8w=
 github.com/cloudwego/iasm v0.2.0/go.mod h1:8rXZaNYT2n95jn+zTI1sDr+IgcD2GVs0nlbbQPiEFhY=
@@ -498,16 +496,6 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO
 github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4=
 github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
 github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
-github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
-github.com/tidwall/gjson v1.14.4 h1:uo0p8EbA09J7RQaflQ1aBRffTR7xedD2bcIVSYxLnkM=
-github.com/tidwall/gjson v1.14.4/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
-github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
-github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
-github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
-github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4=
-github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
-github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY=
-github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20220101234140-673ab2c3ae75 h1:6fotK7otjonDflCTK0BCfls4SPy3NcCVb5dqqmbRknE=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20220101234140-673ab2c3ae75/go.mod h1:KO6IkyS8Y3j8OdNO85qEYBsRPuteD+YciPomcXdrMnk=
 github.com/tredoe/osutil v1.5.0 h1:UGVxbbHRoZi8xXVmbNZ2vgG6XoJ15ndE4LniiQ3rJKg=

lang/ast/structs.go

@@ -5624,7 +5624,7 @@ func (obj *StmtProg) SetScope(scope *interfaces.Scope) error {
 	// debugging visualizations
 	if obj.data.Debug && orderingGraphSingleton {
 		obj.data.Logf("running graphviz for ordering graph...")
-		if err := orderingGraph.ExecGraphviz(context.TODO(), "/tmp/graphviz-ordering.dot"); err != nil {
+		if err := orderingGraph.ExecGraphviz("/tmp/graphviz-ordering.dot"); err != nil {
 			obj.data.Logf("graphviz: errored: %+v", err)
 		}
 		//if err := orderingGraphFiltered.ExecGraphviz("/tmp/graphviz-ordering-filtered.dot"); err != nil {

lang/funcs/dage/dage.go

@@ -961,7 +961,7 @@ func (obj *Engine) Graph() *pgraph.Graph {
 // ExecGraphviz writes out the diagram of a graph to be used for visualization
 // and debugging. You must not modify the graph (eg: during Lock) when calling
 // this method.
-func (obj *Engine) ExecGraphviz(ctx context.Context, dir string) error {
+func (obj *Engine) ExecGraphviz(dir string) error {
 	// No mutex needed here since this func runs in a non-concurrent Txn.
 
 	// No mutex is needed at this time because we only run this in txn's and
@@ -1019,7 +1019,7 @@ func (obj *Engine) ExecGraphviz(ctx context.Context, dir string) error {
 		},
 	}
 
-	if err := gv.Exec(ctx); err != nil {
+	if err := gv.Exec(); err != nil {
 		return err
 	}
 	return nil

lang/interpret_test.go

@@ -567,7 +567,7 @@ func TestAstFunc1(t *testing.T) {
 			}
 			if runGraphviz {
 				t.Logf("test #%d: Running graphviz...", index)
-				if err := fgraph.ExecGraphviz(context.TODO(), "/tmp/graphviz.dot"); err != nil {
+				if err := fgraph.ExecGraphviz("/tmp/graphviz.dot"); err != nil {
 					t.Errorf("test #%d: FAIL", index)
 					t.Errorf("test #%d: writing graph failed: %+v", index, err)
 					return
@@ -1120,7 +1120,7 @@ func TestAstFunc2(t *testing.T) {
 				}
 				ast.ScopeGraph(graph)
 
-				if err := graph.ExecGraphviz(context.TODO(), "/tmp/set-scope.dot"); err != nil {
+				if err := graph.ExecGraphviz("/tmp/set-scope.dot"); err != nil {
 					t.Errorf("test #%d: FAIL", index)
 					t.Errorf("test #%d: writing graph failed: %+v", index, err)
 					return
@@ -1233,7 +1233,7 @@ func TestAstFunc2(t *testing.T) {
 
 			if runGraphviz {
 				t.Logf("test #%d: Running graphviz...", index)
-				if err := fgraph.ExecGraphviz(context.TODO(), "/tmp/graphviz.dot"); err != nil {
+				if err := fgraph.ExecGraphviz("/tmp/graphviz.dot"); err != nil {
 					t.Errorf("test #%d: FAIL", index)
 					t.Errorf("test #%d: writing graph failed: %+v", index, err)
 					return
@@ -1996,7 +1996,7 @@ func TestAstFunc3(t *testing.T) {
 				}
 				ast.ScopeGraph(graph)
 
-				if err := graph.ExecGraphviz(context.TODO(), "/tmp/set-scope.dot"); err != nil {
+				if err := graph.ExecGraphviz("/tmp/set-scope.dot"); err != nil {
 					t.Errorf("test #%d: FAIL", index)
 					t.Errorf("test #%d: writing graph failed: %+v", index, err)
 					return
@@ -2088,7 +2088,7 @@ func TestAstFunc3(t *testing.T) {
 
 			if runGraphviz {
 				t.Logf("test #%d: Running graphviz...", index)
-				if err := fgraph.ExecGraphviz(context.TODO(), "/tmp/graphviz.dot"); err != nil {
+				if err := fgraph.ExecGraphviz("/tmp/graphviz.dot"); err != nil {
 					t.Errorf("test #%d: FAIL", index)
 					t.Errorf("test #%d: writing graph failed: %+v", index, err)
 					return

lib/main.go

@@ -1049,8 +1049,7 @@ func (obj *Main) Run() error {
 						obj.ge.Graph(): nil,
 					},
 				}
-				// FIXME: is this the right ctx?
-				if err := gv.Exec(exitCtx); err != nil {
+				if err := gv.Exec(); err != nil {
 					Logf("graphviz: %+v", err)
 				} else {
 					Logf("graphviz: successfully generated graph!")

pgraph/graphviz.go

@@ -30,7 +30,6 @@
 package pgraph // TODO: this should be a subpackage
 
 import (
-	"context"
 	"fmt"
 	"html"
 	"os"
@@ -154,7 +153,7 @@ func (obj *Graphviz) Text() string {
 
 // Exec writes out the graphviz data and runs the correct graphviz filter
 // command.
-func (obj *Graphviz) Exec(ctx context.Context) error {
+func (obj *Graphviz) Exec() error {
 	filter := ""
 	switch obj.Filter {
 	case "":
@@ -196,7 +195,7 @@ func (obj *Graphviz) Exec(ctx context.Context) error {
 	}
 
 	out := fmt.Sprintf("%s.png", filename)
-	cmd := exec.CommandContext(ctx, path, "-Tpng", fmt.Sprintf("-o%s", out), filename)
+	cmd := exec.Command(path, "-Tpng", fmt.Sprintf("-o%s", out), filename)
 
 	if err1 == nil && err2 == nil {
 		cmd.SysProcAttr = &syscall.SysProcAttr{}
@@ -287,7 +286,7 @@ func (obj *Graph) Graphviz() string {
 
 // ExecGraphviz writes out the graphviz data and runs the correct graphviz
 // filter command.
-func (obj *Graph) ExecGraphviz(ctx context.Context, filename string) error {
+func (obj *Graph) ExecGraphviz(filename string) error {
 	gv := &Graphviz{
 		Graphs: map[*Graph]*GraphvizOpts{
 			obj: nil,
@@ -297,5 +296,5 @@ func (obj *Graph) ExecGraphviz(ctx context.Context, filename string) error {
 		Filename: filename,
 		//Hostname: hostname,
 	}
-	return gv.Exec(ctx)
+	return gv.Exec()
 }

pgraph/pgraph_test.go

@@ -485,7 +485,7 @@ func TestTopoSort3(t *testing.T) {
 	G.AddEdge(v5, v6, e5)
 	G.AddEdge(v4, v2, e6) // cycle
 
-	//G.ExecGraphviz(context.TODO(), "/tmp/g.dot")
+	G.ExecGraphviz("/tmp/g.dot")
 
 	_, err := G.TopologicalSort()
 	if err == nil {