Code Review - PR #3: Configure Grafana with no-auth access and Tilt link

Summary

This PR enables authentication-free Grafana access for local development across all three environments (central, cloud, edge) and adds Tilt UI integration. Overall, the implementation is clean and follows the project's Jsonnet patterns well.

✅ Strengths

1. Consistent Implementation: All three environments follow the same pattern for Grafana configuration
2. Clear Documentation: PR description thoroughly explains the changes and their purpose
3. Proper Use of Common Config: Correctly references common.central.grafana_domain from tanka/lib/common.libsonnet:118
4. Idiomatic Jsonnet: Uses +: and + operators appropriately for merging configurations

🔒 Security Concerns

Critical: Production Deployment Risk

Severity: HIGH

The configuration disables all authentication and grants anonymous Admin role:

'auth.anonymous'+: {
  enabled: true,
  org_role: 'Admin',  // ⚠️ Full admin access without authentication
},
auth+: {
  disable_login_form: true,
},

Issues:

• No environment-based conditional logic to restrict this to dev/local only
• If accidentally deployed to production, this would be a critical security vulnerability
• Admin role grants full write access (create/modify/delete dashboards, datasources, users, etc.)

Recommendations:

1. Add environment awareness to ensure this only applies to local development:

local isLocal = std.extVar('env') == 'local';  // or similar mechanism
'grafana.ini'+: if isLocal then {
  'auth.anonymous'+: { enabled: true, org_role: 'Admin' },
  auth+: { disable_login_form: true },
} else {},

2. Alternatively, document prominently in README/CLAUDE.md that these configs are dev-only
3. Consider using Viewer or Editor role instead of Admin even for dev (least privilege)
4. Add validation in CI to prevent deployment with anonymous auth enabled

🐛 Potential Issues

1. Inconsistent Domain Usage

File: tanka/environments/mop-cloud/kps.jsonnet:17 and tanka/environments/mop-edge/kps.jsonnet:17

Both mop-cloud and mop-edge use common.central.grafana_domain:

hosts: [
  common.central.grafana_domain,  // ← Using "central" domain
],

Expected: Should these environments have their own domain configurations?

• common.cloud.grafana_domain for mop-cloud
• common.edge.grafana_domain for mop-edge

Impact: All environments point to same hostname, which could cause:

• Ingress conflicts if deployed simultaneously
• Confusion about which environment you're accessing
• Issues if environments are on different clusters

Recommendation: Either:

1. Add grafana_domain to common.cloud and common.edge in tanka/lib/common.libsonnet
2. Or document that all local environments intentionally share the same hostname

2. Missing Ingress Class

Files: All three environment files

The ingress configuration doesn't specify an ingressClassName:

ingress+: {
  enabled: true,
  hosts: [...],
  // Missing: ingressClassName
}

Impact:

• Relies on default IngressClass in cluster
• May fail if no default is configured
• Unclear which ingress controller handles these routes

Recommendation:

ingress+: {
  enabled: true,
  ingressClassName: 'nginx',  // or whatever you're using in minikube
  hosts: [...],
}

3. Namespace Inconsistency

File: tanka/environments/mop-cloud/kps.jsonnet:10

namespace: 'monitoring',  // ← Hardcoded

While mop-central and mop-edge use:

namespace: common.namespace,  // ← References common.libsonnet

Impact: Inconsistent patterns across environments; harder to maintain

Recommendation: Either use common.namespace everywhere, or document why mop-cloud needs a different namespace

📝 Code Quality Suggestions

1. Reduce Configuration Duplication

The Grafana auth config is duplicated across all three files. Consider extracting to common.libsonnet:

// In tanka/lib/common.libsonnet
{
  grafana:: {
    noAuthConfig:: {
      'grafana.ini'+: {
        'auth.anonymous'+: {
          enabled: true,
          org_role: 'Admin',
        },
        auth+: {
          disable_login_form: true,
        },
      },
    },
  },
}

Then in each environment:

'grafana.ini'+: common.grafana.noAuthConfig['grafana.ini'],

Benefits: Single source of truth, easier to modify/remove later

2. Incomplete Test Plan

From the PR description:

• Deploy to minikube and verify Grafana accessible without login
• Confirm Tilt link opens Grafana dashboard

Recommendation: Complete manual testing before merge to ensure:

• Ingress routing works correctly
• Anonymous auth actually grants access
• Tilt link resolves properly (requires /etc/hosts entry or DNS)

🎯 Best Practices

1. Missing Host Configuration Documentation

Users need to configure /etc/hosts for grafana.gudo11y.local to work:

# /etc/hosts
127.0.0.1 grafana.gudo11y.local

Recommendation: Add this to PR description or update project README/docs

2. Tiltfile Comment

The comment in Tiltfile:31 could be more descriptive:

# --- links to services ---  # Current
# --- Service Links (accessible in Tilt UI) ---  # Suggested

✨ Performance Considerations

No significant performance concerns. The configuration changes are declarative and don't introduce runtime overhead.

🧪 Test Coverage

Missing: No automated tests for Grafana configuration

Recommendations:

1. Add validation test that checks Grafana config renders correctly:

tk show tanka/environments/mop-central | grep "auth.anonymous"

2. Consider adding to CI pipeline to ensure manifests are valid YAML/JSON

Summary & Verdict

Overall Assessment: Good implementation with clean Jsonnet code, but has critical security concerns that must be addressed.

Required Changes:

1. ⚠️ Add safeguards against accidental production deployment with no-auth
2. Fix domain configuration inconsistency for cloud/edge environments
3. Complete manual testing checklist

Recommended Changes:

1. Extract common Grafana config to shared library
2. Add ingress class specification
3. Document /etc/hosts requirement
4. Consider using less privileged role (Editor instead of Admin)

Approval: Conditional - please address security concerns before merging.

Great work on the consistent implementation across environments! The Tilt integration is a nice DX improvement. 🚀